-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Storage Sync Swagger preview version (prev PR #109 #3211
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Automation for azure-libraries-for-javaThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-pythonThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-rubyNothing to generate for azure-sdk-for-ruby |
Automation for azure-sdk-for-nodeThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-goNothing to generate for azure-sdk-for-go |
| "type": "string" | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to give this a more descriptive name? E.g. 'StorageSyncError'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
| "info": { | ||
| "title": "Microsoft Storage Sync", | ||
| "description": "Microsoft Storage Sync Service API", | ||
| "version": "2017-06-05-preview" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add the client name here? Something like:
"x-ms-code-generation-settings": {
"name": "StorageSyncClient" or "StorageSyncManagementClient"
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
| "ResourceProvider", | ||
| "Operations" | ||
| ], | ||
| "operationId": "OperationGet_list", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The operation ID here should just be "Operations_List"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
| "type": "string" | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use the x-ms-client-name extension to rename this for a better developer experience:
"x-ms-client-name": "pauseWaitForSyncDrainTimePeriodInSeconds"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"pauseWaitForSyncDrainTimePeriodInSeconds": {
"type": "integer",
"description": "Pre Restore pause wait for sync drain time period in seconds."
"x-ms-client-name": "pauseWaitForSyncDrainTimePeriodInSeconds"
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
| "type": "string" | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Operation ID should be in the format group_verb. So this should be changed to:
"operationId": "Workflows_abort"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved. Workflows_Abort
| "tags": [ | ||
| "StorageSyncServices Resource" | ||
| ], | ||
| "operationId": "StorageSyncServiceByResourceGroupGet_list", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Operation ID should be in the format "group_verb". So this should be changed to something like:
"operationId": "StorageSyncServices_ListByResourceGroup"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
| "tags": [ | ||
| "StorageSyncServices Resource" | ||
| ], | ||
| "operationId": "StorageSyncServiceGet_ListBySubscription", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Operation ID should be in the format "group_verb". So this should be changed to something like:
"operationId": "StorageSyncServices_ListBySubscription"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
| "tags": [ | ||
| "SyncGroup Resource" | ||
| ], | ||
| "operationId": "GetSyncGroups_ListByStorageSyncService", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Operation ID should be in the format "group_verb". So this should be changed to something like:
"operationId": "SyncGroups_ListByStorageSyncService"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
| "tags": [ | ||
| "CloudEndpoint Resource" | ||
| ], | ||
| "operationId": "CloudEndpointsGet_ListBySyncGroup", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Operation ID should be in the format "group_verb". So this should be changed to something like:
"operationId": "CloudEndpoints_ListBySyncGroup"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
| "Actions", | ||
| "Backup Restore" | ||
| ], | ||
| "operationId": "CloudEndpointPreBackup_Post", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Operation ID should be in the format "group_verb". So this should be changed to something like:
"operationId": "CloudEndpoints_PreBackup"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
Review comments
StorageSyncError change
|
@ankushbindlish2 - thanks for all the updates! With regards to pausewaitforsyncdraintimeperiodinseconds, the name of the property needs to be the same capitalization as the request over the wire. So something like this: What this does is it will convert the name to the best language-specific capitalization for the generated SDKs. e.g. for Python, the property will be |
extra comma in pauseWaitForSyncDrainTimePeriodInSeconds
body to parameters
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "body", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be renamed to something like "parameters"
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "body", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be renamed to something like "parameters"
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "body", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be renamed to something like "parameters"
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "body", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be renamed to something like "parameters"
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "body", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be renamed to something like "parameters"
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "body", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be renamed to something like "parameters"
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "body", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be renamed to something like "parameters"
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "body", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be renamed to something like "parameters"
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "body", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be renamed to something like "parameters"
| }, | ||
| "headers": { | ||
| "x-ms-request-id": { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be renamed to something like "parameters"
This checklist is used to make sure that common issues in a pull request are addressed. This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.
PR information
api-versionin the path should match theapi-versionin the spec).Quality of Swagger