-
Notifications
You must be signed in to change notification settings - Fork 5.6k
cost-management - API changes #4818
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
…anagement/preview/2018-08-01-preview/costmanagement.json Add more Scopes to Alerts. Also add /updateStatus to Alerts
|
If you're a MSFT employee, click this link |
Automation for azure-sdk-for-jsNothing to generate for azure-sdk-for-js |
Automation for azure-sdk-for-pythonNothing to generate for azure-sdk-for-python |
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-javaNothing to generate for azure-sdk-for-java |
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: |
|
Can one of the admins verify this patch? |
...ent/resource-manager/Microsoft.CostManagement/preview/2018-08-01-preview/costmanagement.json
Show resolved
Hide resolved
KrisBash
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.
Changes seem fine. one question in comments
|
@KrisBash , @praries880 Who can merge the code? as I'm not authorized to merge this pull request |
| }, | ||
| { | ||
| "name": "$filter", | ||
| "description": "May be used to filter alerts by properties/definition/type, properties/definition/category, properties/definition/criteria, properties/costEntityId, properties/creationTime, properties/closeTime, properties/status, properties/source. Supported operators are 'eq','lt', 'gt', 'le', 'ge'.", |
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.
@yaborens
Examples corresponding to this change?
Also, is the list of filters a closed list? If so... why not use an enum?
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.
Examples corresponding to this change? sorry but I don't understand the question
is the list of filters a closed list? If so... why not use an enum? this is how all other groups implemented a filter.
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.
@yaborens
Here are the changes to the example linked here : https://github.com/Azure/azure-rest-api-specs/pull/4818/files/1d5c0565121e5fef4897b32e78318df919406717#diff-cd2cdd86afbc1836331eaef83f213b9cR30
Is that all that changes there for the new API added here?
As for the supported values for the "filter" parameter from the description above : "properties/definition/type, properties/definition/category, properties/definition/criteria, properties/costEntityId, properties/creationTime, properties/closeTime, properties/status, properties/source", is this a list that changes dynamically? If not, then why not model it as an enum?
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.
Hi,
Is that all that changes there for the new API added here? yes
- Filter is not a close group, thus it cannot be enum.
- properties/definition/type - not a close group, thus it cannot be enum.
- properties/definition/category - Already an Enum
- properties/definition/criteria - Already an Enum
- properties/costEntityId - not a close group, thus it cannot be enum.
- properties/creationTime- Not an Enum field
- properties/closeTime - Not an Enum field
- properties/status - Already an Enum
- properties/source - Already an Enum
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.
@yaborensth Some comments
| }, | ||
| { | ||
| "name": "$filter", | ||
| "description": "May be used to filter alerts by properties/definition/type, properties/definition/category, properties/definition/criteria, properties/costEntityId, properties/creationTime, properties/closeTime, properties/status, properties/source. Supported operators are 'eq','lt', 'gt', 'le', 'ge'.", |
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.
@yaborens
Examples corresponding to this change?
Also, is the list of filters a closed list? If so... why not use an enum?
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.
same comments as above
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.
I see only this change in the example linked here : https://github.com/Azure/azure-rest-api-specs/pull/4818/files/1d5c0565121e5fef4897b32e78318df919406717#diff-cd2cdd86afbc1836331eaef83f213b9cR30
Is that all changes in the example for this new API?
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.
yes
| }, | ||
| { | ||
| "name": "$filter", | ||
| "description": "May be used to filter alerts by properties/definition/type, properties/definition/category, properties/definition/criteria, properties/costEntityId, properties/creationTime, properties/closeTime, properties/status, properties/source. Supported operators are 'eq','lt', 'gt', 'le', 'ge'.", |
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.
@yaborens
Examples corresponding to this change?
Also, is the list of filters a closed list? If so... why not use an enum?
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.
same comments as above
| }, | ||
| { | ||
| "name": "$filter", | ||
| "description": "May be used to filter alerts by properties/definition/type, properties/definition/category, properties/definition/criteria, properties/costEntityId, properties/creationTime, properties/closeTime, properties/status, properties/source. Supported operators are 'eq','lt', 'gt', 'le', 'ge'.", |
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.
@yaborens
Examples corresponding to this change?
Also, is the list of filters a closed list? If so... why not use an enum?
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.
same comments as above
* Update specification/cost-management/resource-manager/Microsoft.CostManagement/preview/2018-08-01-preview/costmanagement.json Add more Scopes to Alerts. Also add /updateStatus to Alerts * Add billing parameter to departments * Add alert parameters
…anagement/preview/2018-08-01-preview/costmanagement.json
Add more Scopes to Alerts.
Also, add /updateStatus to Alerts
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.