-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
[PostgreSQL] New api version 2023-03-01-preview - Migrations API property update, enum values update. Network property changes ported. #24087
Conversation
Hi, @ambrahma Thanks for your PR. I am workflow bot for review process. Here are some small tips. Any feedback about review process or workflow bot, pls contact swagger and tools team. [email protected] |
Swagger Generation Artifacts
|
Generated ApiView
|
Hi @ambrahma, one or multiple breaking change(s) is detected in your PR. Please check out the breaking change(s), and provide business justification in the PR comment and @ PR assignee why you must have these change(s), and how external customer impact can be mitigated. Please ensure to follow breaking change policy to request breaking change review and approval before proceeding swagger PR review. |
Added breaking change review => Scenario 23568995: [PostgreSQL] Breaking changes - Add changes to new preview api version for which there is no SDK yet |
…sist in auto-generation of SDK
Hi @ambrahma, Your PR has some issues. Please fix the CI sequentially by following the order of
|
…ons.json to avoid errors" This reverts commit 0171236.
…duplicate entity name errors.
@konrad-jamrozik can we approve the Avocado error? |
Avocado provides guidance how to address failures it reported, here:
@ambrahma can you follow the guidance provided above and if you think it is incorrect or doesn't apply, clarify why? Thanks! |
Guidance says that I need to consult API review team (which is essentially a group I need to join and get approval) to take up this change. I am worried how long will this take overall for approval etc. The fixes in this PR are required for SDK release for api version 2023-03-01-preview and I do not want to miss current cycle. I am also checking with my team if this is ok and what will be implications for the end users who would already be using the same SDK for different services? |
hi @konrad-jamrozik , I have started another PR #24142 for this folder structure change. Basically we want to make sure that same python SDK gets generated for both postgresql single server and postgresql flexible server. This is to avoid downstream issues like letting customers know that there is separate (python SDK) package created for postgresql flexible server vs single server. Also then we need to worry about consuming multiple packages for CLI as currently even our CLI releases are couple together. Moreover currently MySQL single server and MySQL flexible servers are also using same packages. This would be a bigger item. Hence requesting you to allow this current PR to be merged manually, while the issues get ironed out in another PR. Also this will ensure that we do not miss the deadline for the current SDK release and make packages and CLI available to our end users as per our commitments |
Hi @konrad-jamrozik , thanks for reviewing this pr, however, the multiple tag is not introduced in this pr, we support both single server and flexible server so we have two tags for both scenario. Could you kindly help to approve this avocado errors? Many thanks. |
@rkmanda @weshaggard can you advise with priority? |
@ambrahma I'm going to merge your PR but please continue your work on fixing the errors otherwise this will continue to come up and eventually end up blocking you. |
ARM API Information (Control Plane)
Changelog
Add a changelog entry for this PR by answering the following questions:
What's the purpose of the update?
Added fix for network property update from the earlier version to new api version 2023-03-01-preview for which SDK is not yet published.
Migrations api enum property values are being updated from Enabled / Disabled to True / False.
Added 2 new string properties to SecretParameters object for Migrations api.
Added breaking change review => Scenario 23568995: [PostgreSQL] Breaking changes - Add changes to new preview api version for which there is no SDK yet
Contribution checklist (MS Employees Only):
If any further question about AME onboarding or validation tools, please view the FAQ.
ARM API Review Checklist
Otherwise your PR may be subject to ARM review requirements. Complete the following:
Check this box if any of the following apply to the PR so that the label "ARMReview" and "WaitForARMFeedback" will be added by bot to kick off ARM API Review. Missing to check this box in the following scenario may result in delays to the ARM manifest review and deployment.
-[ ] To review changes efficiently, ensure you copy the existing version into the new directory structure for first commit and then push new changes, including version updates, in separate commits. You can use OpenAPIHub to initialize the PR for adding a new version. For more details refer to the wiki. Note that this doesn't apply if you are trying to merge a PR that was previously in the private repository.
Ensure you've reviewed following guidelines including ARM resource provider contract and REST guidelines. Estimated time (4 hours). This is required before you can request review from ARM API Review board.
If you are blocked on ARM review and want to get the PR merged with urgency, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
Breaking Change Review Checklist
If you have any breaking changes as defined in the Breaking Change Policy, request approval from the Breaking Change Review Board.
Action: to initiate an evaluation of the breaking change, create a new intake using the template for breaking changes. Additional details on the process and office hours are on the Breaking Change Wiki.
NOTE: To update API(s) in public preview for over 1 year (refer to Retirement of Previews)
Please follow the link to find more details on PR review process.