Skip to content
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

add new version to opInsights #1807

Closed

Conversation

vipinhas
Copy link
Contributor

No description provided.

@jorgecotillo
Copy link
Contributor

Your service is onboarded in schema auto-generation, I'll look into why the update was not picked up, in theory, once your RP is onboarded, you no longer need to make manual updates

@vipinhas
Copy link
Contributor Author

vipinhas commented Aug 1, 2021 via email

@jorgecotillo
Copy link
Contributor

Thank you Jorge! We updated the swagger a couple of weeks ago and we're about to announce the changes on Aug 4th, do you think this timeline should be ok? Also, can you approve my pr permissions? (I'm a first-time contributor) so we can release this change faster? Missing swagger updates - Vipinhas/Remove Cluster max capacity and Add new Api Version by vipinhas · Pull Request #14953 · Azure/azure-rest-api-specs (github.com)<Azure/azure-rest-api-specs#14953> -removing read only property, no longer in use - NOT a breaking change by vipinhas · Pull Request #13812 · Azure/azure-rest-api-specs (github.com)https://github.com/Azure/azure-rest-api-specs/pull/13812/files Shcemas PR - add new version to opInsights by vipinhas · Pull Request #1807 · Azure/azure-resource-manager-schemas (github.com)<#1807> Best, Viki

________________________________ From: Jorge Cotillo @.> Sent: Saturday, July 31, 2021 2:20 AM To: Azure/azure-resource-manager-schemas @.> Cc: Victoria Pinhasov @.>; Author @.> Subject: Re: [Azure/azure-resource-manager-schemas] add new version to opInsights (#1807) Your service is onboarded in schema auto-generation, I'll look into why the update was not picked up, in theory, once your RP is onboarded, you no longer need to make manual updates — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAzure%2Fazure-resource-manager-schemas%2Fpull%2F1807%23issuecomment-890249770&data=04%7C01%7CVictoria.Pinhasov%40microsoft.com%7Ccea773f51ca94e96cc9208d953b0a313%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632840456820534%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=xeBFEQEnElSegtot7f2IvXi3YINYJVLHVNDw%2FJS1OiE%3D&reserved=0, or unsubscribehttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FATSJB4NYUSWZKJ2ZE4OA5UDT2MXUJANCNFSM5AYEFSKQ&data=04%7C01%7CVictoria.Pinhasov%40microsoft.com%7Ccea773f51ca94e96cc9208d953b0a313%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632840456830529%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=NCmp78TnQZ%2FQDQ6nAk3HUN52NO3cZpDibUvDk2J415s%3D&reserved=0.

There are two issues here:

  1. Capacity enum - you need to update your swagger spec: https://github.com/Azure/azure-rest-api-specs/blob/37fabde7d6f545f2258df22b34f99941520403da/specification/operationalinsights/resource-manager/Microsoft.OperationalInsights/stable/2021-06-01/Clusters.json#L524 and add the default values
  2. Our autorest extension - the one that converts a swagger into schemas - need to understand int enums (I am actually verifying if this is not the case, if it is not, I'll create a PR and fix the extension)

@jorgecotillo
Copy link
Contributor

Also, you mentioned that you want to announce the changes on Aug 4th, are you updating the schemas so the ARM VSCode extension knows how to handle the enums?

@vipinhas
Copy link
Contributor Author

vipinhas commented Aug 3, 2021 via email

@vipinhas
Copy link
Contributor Author

vipinhas commented Aug 3, 2021 via email

@jorgecotillo
Copy link
Contributor

Microsoft docs (REST reference) uses the swagger spec repo, not the schemas repo, so updating this repo would not have any impact in Microsoft docs site.

I am a little confused by your update - w.r.t. ClusterSku - capacity property, if I see here (in rest-api-specs -> main branch):

https://github.com/Azure/azure-rest-api-specs/blob/a8719243647b7c6d06ed287483d788808de7ecab/specification/operationalinsights/resource-manager/Microsoft.OperationalInsights/stable/2021-06-01/Clusters.json#L524

I see capacity being modeled as integer without enum reference (also, I checked yesterday and noticed that currently we don't support integer enums in the schema generation, I'll work on adding the support and expect to be ready in ~1 week).

Lastly, I see that WorkspaceSku - capacityReservationLevel is in fact modeled as integer and references an enum, see here:
https://github.com/Azure/azure-rest-api-specs/blob/37fabde7d6f545f2258df22b34f99941520403da/specification/operationalinsights/resource-manager/Microsoft.OperationalInsights/stable/2021-06-01/Workspaces.json#L422

Your RP is onboarded in autogeneration, so you don't need to make manual updates, the only problem we have is that the schema generation - currently - does not understand integer enums.

@vipinhas
Copy link
Contributor Author

vipinhas commented Aug 8, 2021 via email

@vipinhas
Copy link
Contributor Author

vipinhas commented Aug 19, 2021 via email

@vipinhas
Copy link
Contributor Author

vipinhas commented Aug 24, 2021 via email

@vipinhas
Copy link
Contributor Author

vipinhas commented Aug 24, 2021 via email

@tfitzmac
Copy link
Contributor

@vipinhas - are you looking for an update to the REST API reference or ARM template reference?

@vipinhas
Copy link
Contributor Author

vipinhas commented Aug 24, 2021 via email

@vipinhas
Copy link
Contributor Author

vipinhas commented Aug 24, 2021 via email

@vipinhas
Copy link
Contributor Author

vipinhas commented Aug 25, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants