Skip to content

[Hub Generated] Review request for Microsoft.Consumption to add version stable/2019-10-01 #12822

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 3, 2021
Merged

[Hub Generated] Review request for Microsoft.Consumption to add version stable/2019-10-01 #12822

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
710875f
consumption spec updates
karthikku-2020 Feb 3, 2021
17a3f69
Reverted casing on the etag Property
karthikku-2020 Mar 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion ...ion/consumption/resource-manager/Microsoft.Consumption/stable/2019-10-01/consumption.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4644,6 +4644,11 @@
"description": "Operation type: Read, write, delete, etc.",
"type": "string",
"readOnly": true
},
"description": {
"description": "Description of the operation.",
"type": "string",
"readOnly": true
}
}
}
Expand Down Expand Up @@ -4934,7 +4939,7 @@
"type": "string",
"description": "Resource type."
},
"etag": {
"eTag": {
Copy link
Contributor

@akning-ms akning-ms Mar 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from ARM common definition. looks correct should be etag. and which item in ADO is what you are trying to fix?

Copy link
Contributor Author

@karthikku-2020 karthikku-2020 Mar 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue reported is this

Additional properties not allowed: eTag
Json path: $.value[*].eTag

The property is there in the model. But the error is still showing. So, am suspecting that either swagger is case sensitive or expecting camel casing on the property. That's the fix am trying to make.

Link to the error - https://portal.azure-devex-tools.com/amekpis/correctness/detail?errorId=43A64D49-5B20-4860-BB81-17D1968E1E0C

Copy link
Contributor

@akning-ms akning-ms Mar 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. it is case-sensitive, in this case, I will recommend you to fix it in your service side. BTW, how about other correctness in ADO items? will you also fix?

Copy link
Contributor Author

@karthikku-2020 karthikku-2020 Mar 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering why this would be a fix on the service side. The response from the service is correct. The casing needs to be updated on the swagger specs. Hence this fix.

For the other issues in the ADO item - Not all the APIs mentioned in that work item is owned by my team. There are multiple teams within cost management that own different APIs. Each team will will own the fixes for their APIs.

Copy link
Contributor

@akning-ms akning-ms Mar 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per ARM guide: https://github.com/Azure/azure-resource-manager-rpc/blob/master/v1.0/resource-api-reference.md
You can find etag in the document. it should be etag, not eTag.

Copy link
Contributor Author

@karthikku-2020 karthikku-2020 Mar 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Reverted the change on the casing update for etag. Are we good with the other change?

Copy link
Contributor

@akning-ms akning-ms Mar 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM,

"readOnly": true,
"type": "string",
"description": "Resource etag."
Expand Down