-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Introducing New Version 2018-10-01 for Consumption API #4069
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-sdk-for-jsThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-pythonThe initial PR has been merged into your service PR: |
|
Can one of the admins verify this patch? |
Automation for azure-sdk-for-javaNothing to generate for azure-sdk-for-java |
Automation for azure-sdk-for-rubyA 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: |
Automation for azure-sdk-for-nodeNothing to generate for azure-sdk-for-node |
Automation for azure-sdk-for-goThe initial PR has been merged into your service PR: |
|
Adding @ravbhatnagar for ARM feedback. |
|
ping @ravbhatnagar |
ravbhatnagar
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.
Looks good. Signing off from ARM.
| "type": "string" | ||
| } | ||
| }, | ||
| "excludedSubscriptions": { |
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.
What are the excluded subscriptions based on?
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.
Checked with Parisa, currently the feature is only supported for EA subs. The excluded subs will show the subs part of management group which are non-EA
|
@parisa-naeimi - could you please update the readme tags to include the new API version? |
|
@annatisch Could you please let me know what do you mean by updating readme tags? do you mean I need to update readme.md under resource-manager and if so, do I need to do it manually? |
|
Thanks @parisa-naeimi - yes that's correct: You can create a new "Tag: package-2018-10" with the path to your new API file. In addition to above, each "multi API" SDK will need a specific tag setup: |
|
@annatisch Thank you very much for providing information. I updated read me tags, please let me know if I need to revisit something. |
|
Thanks @parisa-naeimi - could you please update them for all languages (or at least - every language for which you have a published SDK)? :) |
|
@annatisch I think I already have added the version to readme files. I do not know what are you referring here or at least let me know what I am missing here. |
|
Thanks @parisa-naeimi - In commit 81df0a1, you have only added a tag for the Java multi-api SDK. So first, add a generic tag at the top of the file, like these ones: Next, update the tag used for standard code gen in the Basic Information at the top here: Finally, in the same way that you just added a "tag and java", you will need to add a "tag and go" and tag and ruby" sections as well. Then you will need to list those new tags under each language - for example here in java: I hope this helps... indeed it is a bit complicated. |
|
@annatisch thank you very much. I think I missed those areas in my first commit. I got it now and updated the readme file. Please let me know if it is good now. |
|
Thanks @parisa-naeimi! The changes look good - but it looks like there is a conflict, could you rebase against master? |
# Conflicts: # specification/consumption/resource-manager/readme.md
|
@annatisch I got the latest from master branch and resolved conflict. I think we are good now. Please let me know if I need to do something in my side. Thank you very much :) |
annatisch
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.
Just a couple of tiny things then it's good to go :)
| { | ||
| "swagger": "2.0", | ||
| "info": { | ||
| "version": "2018-08-31", |
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.
Please update the version tag to "2018-10-01"
| } | ||
| }, | ||
| "x-ms-pageable": { | ||
| "nextLinkNa |
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.
Please make "subscriptionGuids" two words: "subscription GUIDs"
| } | ||
| }, | ||
| "x-ms-pageable": { | ||
| "nextLinkNa |
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.
Please make "subscriptionGuids" two words: "subscription GUIDs"
|
@annatisch Hello, thank you for the feedback, I applied all of them. Please let me know if I need to update it again. |
|
Thanks @parisa-naeimi - Line 4302 in 63c2d0f
|
|
@annatisch I kept it as it is just updated the version. Thanks for catching it. |
|
@annatisch Hello Again, you merged these changes 4 days ago but I do not know I cannot see the new version of consumption and also my recent changes. Any suggestion here? |
|
@parisa-naeimi - I'm not sure I understand - your changes have been merged here: Or did you mean your changes in each specific language SDK? |
|
When I go to this link for consumption rp https://docs.microsoft.com/en-us/rest/api/consumption/ I supposed to see the latest version which is 2018-10-01 but I still see the previous version which is 2018-08-31. It does not seem right? |
|
I don't think the updated API docs are automatic. |
|
@annatisch I have done this before and always after merge I used to see update automatically. |
|
I do not know how API docs are generated from swagger specs - please reach out to the docs team. |
|
@annatisch looks like I need to reach out to them since it is a version change. Thank you for the help |
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