-
Notifications
You must be signed in to change notification settings - Fork 5.6k
CosmosDB autopilot support #8428
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
|
Azure Pipelines successfully started running 1 pipeline(s). |
azure-sdk-for-net - Release
|
azure-sdk-for-go - Release⌛ Pending...
|
azure-sdk-for-js - Release
|
azure-sdk-for-java - Release
|
azure-sdk-for-python - Release
|
|
@ravgill This spec needs a .Net readme file. Please add this. |
markcowl
left a comment
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.
Failing validation because there is no .net readme file for this spec - please add a .Net readme file.
|
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@markcowl can you please re-review this? |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| }, | ||
| "required": [ | ||
| "throughput" | ||
| "maxThroughput" |
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.
It looks like 'throughput is now not required for 'ThroughpoutSettingsResource', is that intentional, is this no longer a required property?
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.
Either "throughput" is required or "autopilotSettings" settings is required. Is there a some way to convey "requiredOneOf" @markcowl
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.
Unfortunately, not until Swagger 3. It is not a breakign change to remove 'required', but I would suggest documenting the fact that one of these are required in the description of the object and in the description for both parameters
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.
@markcowl I have updated the description of the object and in the description for both parameters
|
@ravgill One question, otherwise lgtm |
|
Azure Pipelines successfully started running 1 pipeline(s). |
* CosmosDB autopilot support * add readme.csharp.md * updated description in ThroughputSettingsResource
Latest improvements:
MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.
Contribution checklist:
ARM API Review Checklist
Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.
Please follow the link to find more details on API review process.