Skip to content

Conversation

@najams
Copy link
Contributor

@najams najams commented Feb 9, 2018

  • Swagger for job
  • Fixed spec for runbook
  • Added examples for runbook
  • Ran validation tools

@AutorestCI
Copy link

AutorestCI commented Feb 9, 2018

Automation for azure-sdk-for-python

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-python#1959

@AutorestCI
Copy link

AutorestCI commented Feb 9, 2018

Automation for azure-sdk-for-go

Encountered a Subprocess error: (azure-sdk-for-go)

Command: profileBuilder -s list -l ./profiles/2017-03-09/defintion.txt -name 2017-03-09
Finished with return code 1
and output:

Error: unknown command "2017-03-09" for "profileBuilder"
Run 'profileBuilder --help' for usage.
unknown command "2017-03-09" for "profileBuilder"

@azuresdkci azuresdkci assigned sergey-shandar and unassigned dsgouda Feb 9, 2018
@sergey-shandar sergey-shandar added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Feb 12, 2018
@vrdmr
Copy link
Member

vrdmr commented Feb 12, 2018

@najams - Could you please take a look at the error that the Python build faced - #2466 (comment)

@azuresdkciprbot
Copy link

Hi There,

I am the AutoRest Linter Azure bot. I am here to help. My task is to analyze the situation from the AutoRest linter perspective. Please review the below analysis result:

File: specification/automation/resource-manager/readme.md
Before the PR: Warning(s): 0 Error(s): 38
After the PR: Warning(s): 0 Error(s): 0

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

@azuresdkciprbot
Copy link

Hi There,

I am the AutoRest Linter Azure bot. I am here to help. My task is to analyze the situation from the AutoRest linter perspective. Please review the below analysis result:

File: specification/automation/resource-manager/readme.md
Before the PR: Warning(s): 0 Error(s): 36
After the PR: Warning(s): 0 Error(s): 24

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

@vrdmr
Copy link
Member

vrdmr commented Feb 14, 2018

Hi @ravbhatnagar, @sergey-shandar - Could you please take a look at this PR. Thanks.

"format": "date-time",
"description": "The end time of the job."
},
"lastModifiedTime": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

read only

"format": "date-time",
"description": "The start time of the job."
},
"endTime": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

read only

"format": "date-time",
"description": "The last modified time of the job."
},
"provisioningState": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

read only

"$ref": "./definitions.json#/parameters/clientRequestId"
}
],
"responses": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this return? does it not have a defined schema.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It returns the output from the script as generated by Powershell. For example, a script with only statement as { get-date } will generate the following output: Saturday, February 17, 2018 10:09:38 PM

whereas, the script like { "hello"; get-date; "bye"; } will generate the output:
hello

Saturday, February 17, 2018 10:11:15 PM
bye

"$ref": "./definitions.json#/parameters/clientRequestId"
}
],
"responses": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

schema?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be a string which represents the script used to run the job. As above, there is no schema as it is a script.

"produces": [
"application/json"
"application/json",
"text/plain",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you really returning these content types from the ARM control plane? text/plain and text/powershell. Please note that only json is supported.

Copy link
Contributor Author

@najams najams Feb 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, these are for the scripts and output as commented above. These are for read-only properties. These APIs are part of ARM since day 1 and signed off by ARM team like Charles, Anders, etc. I only fixed the swagger. Also, these are not used in ARM templates.

"description": "The Resource definition.",
"x-ms-azure-resource": true
},
"ProvisioningStateProperty": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enum.

}
},
"produces": [
"text/powershell"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

content type is an issue

@najams
Copy link
Contributor Author

najams commented Feb 19, 2018

Hi @ravbhatnagar, I have addressed your feedback. Please review and let me know if you have more questions. Overall, these are existing GA APIs out for several years. Thanks.

@ravbhatnagar
Copy link
Contributor

Signing off since these are existing APIs. ARM content type validation needs to be enabled going forward to disallow such behavior.

@ravbhatnagar ravbhatnagar added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review Conditionally-Merged and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Feb 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review Conditionally-Merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants