Skip to content

[Compute] Add Gallery scripts specs #33511

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

Jump to bottom
Open
wants to merge 16 commits into
base: feature/cplat-2025-03-03
Choose a base branch
from
Open

[Compute] Add Gallery scripts specs #33511

wants to merge 16 commits into from

Conversation

viveklingaiah
Copy link
Contributor

@viveklingaiah viveklingaiah commented Mar 26, 2025 •
edited
Loading

===

Choose a PR Template

Switch to "Preview" on this description then select one of the choices below.

Click here to open a PR for a Control Plane (ARM) API.

@openapi-pipeline-app openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Mar 26, 2025 •
edited
Loading

Next Steps to Merge

Next steps that must be taken to merge this PR:
  • ❌ Your PR has breaking changes in the generated SDK for Go (label: BreakingChange-Go-Sdk). Refer to step 3 in the PR workflow diagram.
  • ❌ Your PR has breaking changes in the generated SDK for JavaScript (label: BreakingChange-JavaScript-Sdk). Refer to step 3 in the PR workflow diagram.

@openapi-pipeline-app openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Mar 26, 2025 •
edited
Loading

PR validation pipeline restarted successfully. If there is ApiView generated, it will be updated in this comment.

@github-actions github-actions bot added the brownfield Brownfield services will soon be required to convert to TypeSpec. See https://aka.ms/azsdk/typespec. label Mar 26, 2025
@viveklingaiah viveklingaiah mentioned this pull request Mar 26, 2025
Closed
@azure-sdk azure-sdk mentioned this pull request Mar 26, 2025
Draft
This was referenced Mar 26, 2025
Draft
Draft
Draft
Draft
@grizzlytheodore grizzlytheodore added ARMReview WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Mar 26, 2025
@viveklingaiah viveklingaiah mentioned this pull request Mar 26, 2025
Open
@azure-sdk azure-sdk mentioned this pull request Mar 26, 2025
Draft
@ramoka178
Copy link
Contributor

@viveklingaiah Please fix the Swagger Avocado and Swagger Lintdiff errors and place it back in queue for review.

@ramoka178 ramoka178 added the ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review label Mar 26, 2025
@openapi-pipeline-app openapi-pipeline-app bot removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Mar 26, 2025
@grizzlytheodore grizzlytheodore added WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required and removed ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review labels Apr 14, 2025
@rkmanda rkmanda added the ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review label Apr 19, 2025
@openapi-pipeline-app openapi-pipeline-app bot removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Apr 19, 2025
@grizzlytheodore grizzlytheodore added WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required and removed ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review labels Apr 21, 2025
@gary-x-li
Copy link
Contributor

Can you please address the two latest comments regarding headers for PUT?

@gary-x-li gary-x-li added the ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review label Apr 22, 2025
@openapi-pipeline-app openapi-pipeline-app bot removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Apr 22, 2025
@viveklingaiah
Copy link
Contributor Author

Can you please address the two latest comments regarding headers for PUT?

Yes, addressed.

@gary-x-li
Copy link
Contributor

The description of the Location header you added is "Geo-location (region) of the resource". This is not correct. Please make sure you go through the RPC for async operations below; and make sure your service behaves the same way as how this is defined in swagger.

https://github.com/cloud-and-ai-microsoft/resource-provider-contract/blob/master/v1.0/async-api-reference.md#creating-replacing-or-updating-resources-asynchronously

In particular, if you are following the async 200/201 PUT pattern, the Location header is not relevant (since it's for the 202 Accepted pattern). The Azure-AsyncOperation header is optional. You should make sure the swagger definition is consistent with what your service would respond.

@viveklingaiah
Copy link
Contributor Author

The description of the Location header you added is "Geo-location (region) of the resource". This is not correct. Please make sure you go through the RPC for async operations below; and make sure your service behaves the same way as how this is defined in swagger.

https://github.com/cloud-and-ai-microsoft/resource-provider-contract/blob/master/v1.0/async-api-reference.md#creating-replacing-or-updating-resources-asynchronously

In particular, if you are following the async 200/201 PUT pattern, the Location header is not relevant (since it's for the 202 Accepted pattern). The Azure-AsyncOperation header is optional. You should make sure the swagger definition is consistent with what your service would respond.

I think addressed this time. Please check

@grizzlytheodore grizzlytheodore added WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required and removed ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review labels Apr 24, 2025
@gary-x-li gary-x-li added the ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review label Apr 24, 2025
@openapi-pipeline-app openapi-pipeline-app bot removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Apr 24, 2025
kazrael2119
kazrael2119 reviewed May 12, 2025
View reviewed changes
...fication/compute/resource-manager/Microsoft.Compute/GalleryRP/stable/2025-03-03/gallery.json
"required": true,
"type": "string",
"pattern": "^[a-zA-Z0-9]+([_]?[a-zA-Z0-9]+)*$",
"description": "The name of the Shared Image Gallery where Image Definitions or other artifacts reside."
Copy link
Contributor

@kazrael2119 kazrael2119 May 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

please add "x-ms-parameter-location": "method" for this parameter definition

kazrael2119
kazrael2119 reviewed May 12, 2025
View reviewed changes
...fication/compute/resource-manager/Microsoft.Compute/GalleryRP/stable/2025-03-03/gallery.json
"required": true,
"type": "string",
"pattern": "^[a-zA-Z0-9]+([_]?[a-zA-Z0-9]+)*$",
"description": "The name of the gallery Script Definition in which the Script Version is to be created."
Copy link
Contributor

@kazrael2119 kazrael2119 May 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

please add "x-ms-parameter-location": "method" for this parameter definition

kazrael2119
kazrael2119 reviewed May 12, 2025
View reviewed changes
...fication/compute/resource-manager/Microsoft.Compute/GalleryRP/stable/2025-03-03/gallery.json
"required": true,
"type": "string",
"pattern": "^[0-9]+\\.[0-9]+\\.[0-9]+$",
"description": "The name of the gallery Script Version to be created. Needs to follow semantic version name pattern: The allowed characters are digit and period. Digits must be within the range of a 32-bit integer. Format: <MajorVersion>.<MinorVersion>.<Patch>"
Copy link
Contributor

Choose a reason for hiding this comment

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

please add "x-ms-parameter-location": "method" for this parameter definition

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gary-x-li gary-x-li gary-x-li left review comments

@rkmanda rkmanda rkmanda left review comments

@ramoka178 ramoka178 ramoka178 left review comments

@kazrael2119 kazrael2119 kazrael2119 left review comments

@razvanbadea-msft razvanbadea-msft razvanbadea-msft left review comments

@jscalev jscalev jscalev approved these changes

@D1v38om83r D1v38om83r D1v38om83r approved these changes

@Sandido Sandido Awaiting requested review from Sandido

@dkulkarni-ms dkulkarni-ms Awaiting requested review from dkulkarni-ms dkulkarni-ms is a code owner

@haagha haagha Awaiting requested review from haagha haagha is a code owner

@MS-syh2qs MS-syh2qs Awaiting requested review from MS-syh2qs MS-syh2qs is a code owner

@grizzlytheodore grizzlytheodore Awaiting requested review from grizzlytheodore grizzlytheodore is a code owner

@mabhard mabhard Awaiting requested review from mabhard mabhard is a code owner

Assignees
No one assigned
Labels
ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review BreakingChange-Go-Sdk BreakingChange-JavaScript-Sdk brownfield Brownfield services will soon be required to convert to TypeSpec. See https://aka.ms/azsdk/typespec. Compute resource-manager
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.