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

Merge release api management 2023 09 01 preview - Active #29184

Open
wants to merge 35 commits into
base: main
Choose a base branch
from

Conversation

solankisamir
Copy link
Member

@solankisamir solankisamir commented May 21, 2024

ARM (Control Plane) API Specification Update Pull Request

Tip

Overwhelmed by all this guidance? See the Getting help section at the bottom of this PR description.

PR review workflow diagram

Please understand this diagram before proceeding. It explains how to get your PR approved & merged.

spec_pr_review_workflow_diagram

Purpose of this PR

What's the purpose of this PR? Check the specific option that applies. This is mandatory!

  • New resource provider.
  • New API version for an existing resource provider. (If API spec is not defined in TypeSpec, the PR should have been created in adherence to OpenAPI specs PR creation guidance).
  • Update existing version for a new feature. (This is applicable only when you are revising a private preview API version.)
  • Update existing version to fix OpenAPI spec quality issues in S360.
  • Other, please clarify:
    • edit this with your clarification

Due diligence checklist

To merge this PR, you must go through the following checklist and confirm you understood
and followed the instructions by checking all the boxes:

  • I confirm this PR is modifying Azure Resource Manager (ARM) related specifications, and not data plane related specifications.
  • I have reviewed following Resource Provider guidelines, including
    ARM resource provider contract and
    REST guidelines (estimated time: 4 hours).
    I understand this is required before I can proceed to the diagram Step 2, "ARM API changes review", for this PR.

Additional information

Viewing API changes

For convenient view of the API changes made by this PR, refer to the URLs provided in the table
in the Generated ApiView comment added to this PR. You can use ApiView to show API versions diff.

Suppressing failures

If one or multiple validation error/warning suppression(s) is detected in your PR, please follow the
suppressions guide to get approval.

Getting help

  • First, please carefully read through this PR description, from top to bottom. Please fill out the Purpose of this PR and Due diligence checklist.
  • To understand what you must do next to merge this PR, see the Next Steps to Merge comment. It will appear within few minutes of submitting this PR and will continue to be up-to-date with current PR state.
  • For guidance on fixing this PR CI check failures, see the hyperlinks provided in given failure
    and https://aka.ms/ci-fix.
  • For help with ARM review (PR workflow diagram Step 2), see https://aka.ms/azsdk/pr-arm-review.
  • If the PR CI checks appear to be stuck in queued state, please add a comment with contents /azp run.
    This should result in a new comment denoting a PR validation pipeline has started and the checks should be updated after few minutes.
  • If the help provided by the previous points is not enough, post to https://aka.ms/azsdk/support/specreview-channel and link to this PR.

solankisamir and others added 12 commits January 3, 2024 07:35
…023-05-01-preview to version 2023-09-01-preview
* WIP

* Update sample

* WIP

* Add GatewayNameParameter

* Clean up

* Update sample

* Add another sample

* Add sample for delete gateway

* Remove endpoints

* Fix typo

* Add list sample

* Clean up

Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>

* Add disclaimer

Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>

* Make it pretty 💄

Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>

* Refer to new file

Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>

* Add missing ApiManagementGatewayListResult

Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>

* Update sample reference

Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>

* Remove ApiManagementGatewayIdentity + List by operation

Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>

* Add more sample

Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>

* Latest version of common types

Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>

* Remove 200 for delete

Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>

* Formatting and re-order

Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>

* Align with spec

Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>

* Align sample with spec

Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>

* Error response from common types

Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>

* Rename SKU name

* Have seperate SKU for patch to not make it mandatory

* Remove empty required

* Improve note

* Rename schema

* Remove unused type

* sku.capacity should not be required

---------

Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>
)

* Add loggers to workspaces

* Add diagnostics to workspace

* Add certificates to workspace

* Add backends to workspaces

* Change DiagnosticContract to DiagnosticContractProperties in patch

* Revert back DiagnosticContractProperties to DiagnosticContract

* Fix CI and add files to readme

* Fix example

* remove reconnect endpoint

* Add suppressions

* Try different suppression where

* Try different suppression

* Try other suppression

* another test

* finally working suppression

* remove unwanted line
* md cb contract

* modify pool contract

* correct the limits

---------

Co-authored-by: Samir Solanki <samirsolanki@outlook.com>
* add operation statuses resource

* fix api-version
* add operation results

* fix spec

* operationresults

* fix file name
* new skutype

* virtualNetworkType

* remove default from patch

* List skus API

* add model for sku

* caps enums
* added gateway config resource

* removing Head gateway Config call

* updates

* resolving errors

* trying to resolve more model validations

* resolving some model validations

* resolving model validations

* resoloving delete example errors

* Swagger Prettier

* prettier

* updating so patch has same response as get and put

* removing path for gateway config

* addressing comment

* addressing comments

* nit

* path updatess

* updating as per comments

* Adding workspaceLinks

* resolving some of the errors

* resolved some errors

* resolving some of the errors

* resolving some more errors

* npx prettier

* addressing comments

* updated based on comments

* lint updates

* nit updates

---------

Co-authored-by: Samir Solanki <samirsolanki@outlook.com>
Copy link

openapi-pipeline-app bot commented May 21, 2024

PR validation pipeline restarted successfully. This comment will be populated with next steps to merge this PR once validation is completed. Please wait ⌛.

Copy link

openapi-pipeline-app bot commented May 21, 2024

PR validation pipeline restarted successfully. This comment will be populated with the 'Swagger Validation Report'

Copy link

openapi-pipeline-app bot commented May 21, 2024

PR validation pipeline restarted successfully. This comment will be populated with the 'Swagger Generation Artifacts' report

Copy link

openapi-pipeline-app bot commented May 21, 2024

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

@solankisamir solankisamir added the 2023-09-01-apim Changes targetting 2023-09-01-preview for API Management label May 21, 2024
@solankisamir solankisamir changed the title Merge release api management 2023 09 01 preview Merge release api management 2023 09 01 preview - Active May 21, 2024
@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 May 30, 2024
@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 May 30, 2024
@solankisamir
Copy link
Member Author

solankisamir commented May 30, 2024

Thanks @gary-x-li, @tomkerkhove can you work with the SDK for Go team to get signoff on
https://github.com/Azure/azure-rest-api-specs/pull/29184/checks?check_run_id=25609156492

[ERROR] error | PreCheck/DuplicateSchema | Duplicate Schema named 'ErrorResponse' (4 differences):

We should be good to merge then.

@solankisamir
Copy link
Member Author

/pr RequestMerge

@microsoft-github-policy-service microsoft-github-policy-service bot added the MergeRequested Part of the ARM PR review workflow label May 31, 2024
@tomkerkhove
Copy link
Member

@solankisamir This will be fixed once we tackle #27012 for our next stable API version

@gary-x-li
Copy link
Contributor

Can you please post a message in this channel regarding the azure-sdk-for-go failure? It's blocking the merge.

@tomkerkhove
Copy link
Member

@gary-x-li This was already approved as part of #27757

@Alancere
Copy link
Member

Alancere commented Jun 3, 2024

Hi @solankisamir, please fix Go CI issue: Duplicate Schema named 'ErrorResponse',
"$ref": "../../../../../common-types/resource-management/v5/types.json#/definitions/ErrorResponse" conflicts with ErrorResponse

@solankisamir
Copy link
Member Author

Hi @solankisamir, please fix Go CI issue: Duplicate Schema named 'ErrorResponse', "$ref": "../../../../../common-types/resource-management/v5/types.json#/definitions/ErrorResponse" conflicts with ErrorResponse

@tomkerkhove can you take a look at this?

@tomkerkhove
Copy link
Member

@Alancere The fix is in.

Go is now failing with the following error, but I have no clue how this name is determined:

client APIManagementGatewayClient was renamed to GatewayClient which collides with an existing client name

Likely this is because we already have a Gateway subresource of service which is unrelated to I'd say we need to tell the SDK to use another name than GatewayClient for our new top-level resource. Any guidance on this would be appreciated.

@Alancere
Copy link
Member

Alancere commented Jun 3, 2024

@Alancere The fix is in.

Go is now failing with the following error, but I have no clue how this name is determined:

client APIManagementGatewayClient was renamed to GatewayClient which collides with an existing client name

Likely this is because we already have a Gateway subresource of service which is unrelated to I'd say we need to tell the SDK to use another name than GatewayClient for our new top-level resource. Any guidance on this would be appreciated.

When generating the GO SDK, the service name prefix(apimanagement) is ​​removed, so APIManagementGateway will be renamed to Gateway, causing a conflict. Please use a better name.

@tomkerkhove
Copy link
Member

tomkerkhove commented Jun 3, 2024

Commit 8d4753d fixes it but we have to rename the operation IDs which are used to define this after talking to @Alancere.

@solankisamir I have reverted this as we did not discuss it but please have a look at the suggestion and apply the commit again or use another prefix. We cannot use Gateway_ either given self-hosted gateway already relies on this.

I have already updated GatewayConfigConnection, though, to remove the ApiManagement prefix in 3e8197f as this is not supposed to be added

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2023-09-01-apim Changes targetting 2023-09-01-preview for API Management Approved-Suppression ARMReview ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review BreakingChange-Approved-BugFix Changes are to correct the REST API definition to correctly describe service behavior BreakingChange-Python-Sdk BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required MergeRequested Part of the ARM PR review workflow new-api-version ReadyForApiTest <valid label in PR review process>add this label when swagger and service APIs are ready for test resource-manager SuppressionReviewRequired
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet