-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Added reservation recommendations and tags #2679
Conversation
Added Tags filter for budgets and updated the api version
Updated comments
Incorporated review comments
Incorporated review comments
Incorporated review comment
Added Tags filter and grouping for UsageDetails
Added reservation recommendations and tags
Automation for azure-libraries-for-javaNothing to generate for azure-libraries-for-java |
Automation for azure-sdk-for-nodeNothing to generate for azure-sdk-for-node |
Automation for azure-sdk-for-pythonThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-goA 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: |
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.
Also note that the addition of the '$apply' parameter will introduce a breaking change for some SDKs.
"$ref": "./examples/ReservationRecommendationsFilterByScopeLookBackPeriod.json" | ||
} | ||
}, | ||
"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.
The subscriptionId
parameter is missing which is the cause of the travis failure, see the log.
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.
Sure! Just missed it. I have added it.
"tags": [ | ||
"ReservationRecommendations" | ||
], | ||
"operationId": "reservationRecommendations_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.
Should be "ReservationRecommendations_List" (upper-case 'r').
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.
Sure! Corrected.
"recommendedQuantity": 1, | ||
"totalCostWithReservedInstances": 0.0, | ||
"netSavings": 4.634521202630137, | ||
"firstUsageDate": "2018-03-06", |
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 schema defines this as date/time however you've only provided a date; this is causing model validation failures, see the log. This is applicable to other samples as well.
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.
Thanks! for sharing this. I have changed the format to DateTime in example.
Incorporated review comments
Review comments have been incorporated. Please take a look now. |
Incorporated review comments
"firstUsageDate": { | ||
"description": "The usage date for looking back.", | ||
"type": "string", | ||
"format": "date-time", |
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.
There are still model validation failure regarding this date-time, see the log. Is this supposed to be just date like the others you changed?
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.
Sorry! Typo error. There was a space in the datetime string example. Just corrected it and waiting for build to complete.
Removed unwanted space
"recommendedQuantity": 1, | ||
"totalCostWithReservedInstances": 0.0, | ||
"netSavings": 4.634521202630137, | ||
"firstUsageDate": "2018-03-06T00:00:00Z ", |
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.
Another extra whitespace, looks like there's on line 42 as well. Please scrub the remaining example files for this error.
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.
Done! I have verified the other files as well.
Removed extra whitespace
@AutorestCI rebuild azure-sdk-for-go |
Please merge the PR if everything looks good. Thank you! |
Adding arczoneid properties for the resources...
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-version
in the path should match theapi-version
in the spec).Quality of Swagger