Azure / azure-sdk-tools

Tools repository leveraged by the Azure SDK team.
MIT License
109 stars 166 forks source link

Management plane: Naming Review - Azure DevEx Arch Board #4601

Open ladonnaq opened 1 year ago

ladonnaq commented 1 year ago

Let's use this GitHub issue to track the outstanding requirements for automation of the naming process for initial management plane libraries.

Status on support of naming approval for management plane via APIView per @praveenkuttappan

API reviews for management packages are automatically generated now for C#, Java and JS but not for Python. We should consider the following before we enable release check to enforce "Approval for first preview" for management packages.

Even though API reviews are already getting generated for few languages, we do not enforce approval of these API reviews and allow management packages to be released without API approval. Enabling approval status check for management package in APIView (approval for first preview release) by release pipeline will require architects to approve any new package that goes out as preview. Approval is not required if package was already released as GA before.

More API reviews will get created and visible in APIView for Python once we enable api review gen for management for Python.

Changes required for APIView:

I assume API approval status check, which is done by architects after reviewing all APIs, is not in the scope of this change and feature request here is to check only for preview version basic approval like package name is valid etc.

Arch Board management plane naming process per @ronniegeraghty

Release Planner requirements

Dashboard/Reporting

Interim TODOs (until we can execute plan above)

ladonnaq commented 1 year ago

@kyle-patterson please review the scenarios that require naming review and edit accordingly. Also, do you think a new meeting type is needed? We could also have a form in the release planner app that takes as input the suggested namespaces, then it could create a GitHub issue which is assigned to the language architects for asynchronous review. If a review was needed the architects could add sync label (similar to what we discussed yesterday for the SDK reviews).

ladonnaq commented 1 year ago

@kyle-patterson and @ronniegeraghty I would like to get this feature in the release planner app. Would you please review and provide more detail so I can hand it over to engineering. The release planner can automatically create the GitHub issue, put it in a GitHub project for you if GitHub action is enabled, and add the appropriate labels.

ladonnaq commented 1 year ago

There is work in progress that is a dependency for the release planner for the naming feature. The release planner needs to add a step to the SDK release plan and a blade so that users can create a GitHub issue for naming.

Work in progress or recently complete in APIView

RickWinter commented 1 year ago

@ladonnaq Can this work be expanded to have APIView require C/C++/GO to approve the name on first beta release. I'd like to avoid shipping unapproved package names in the those languages as well.

maririos commented 1 year ago

@ladonnaq as I mentioned in our Teams channel, according to the requirements asked from us, all implementation from APIView has been done. @RickWinter this works for all languages that support APIView, same as API aprovals

maririos commented 1 year ago

Correction on my statement: This is only true for data plane libraries.

There is no support for naming approval or API approval for management plane libraries. The later was a request for architects as the libraries are mostly autogenerated. If this request changes, please involve @mikekistler as the APIView PM

RickWinter commented 1 year ago

@maririos , @mikekistler , @ladonnaq - We need this naming validation for control plane as well. That is where the two most recent mistakes have occurred.

ladonnaq commented 1 year ago

@maririos , @mikekistler , @ladonnaq - We need this naming validation for control plane as well. That is where the two most recent mistakes have occurred.

I just found out management plane is not supported. I opened a GitHub issue and tagged @RickWinter and assigned to @mikekistler https://github.com/Azure/azure-sdk-tools/issues/4922

ladonnaq commented 1 year ago

@maririos @mikekistler Have there been any discussions around how to support management plane naming request in APIView as well? If management plane will not be supported for approving naming, then we should have the release planner gather all of the necessary information to create the GitHub issue and use labels to track approval of the each package. Here is the manual process today for naming - https://dev.azure.com/azure-sdk/internal/_wiki/wikis/internal.wiki/821/Naming-for-new-initial-management-or-client-libraries-(new-SDKs)

maririos commented 1 year ago

Meeting scheduled to discuss this topic.

maririos commented 1 year ago

@praveenkuttappan could you provide the requirements and implications for this feature to be done?

praveenkuttappan commented 1 year ago

API reviews for management packages are automatically generated now for C#, Java and JS but not for Python. We should consider the following before we enable release check to enforce "Approval for first preview" for management packages.

  1. Even though API reviews are already getting generated for few languages, we do not enforce approval of these API reviews and allow management packages to be released without API approval. Enabling approval status check for management package in APIView (approval for first preview release) by release pipeline will require architects to approve any new package that goes out as preview. Approval is not required if package was already released as GA before.

  2. More API reviews will get created and visible in APIView for Python once we enable api review gen for management for Python.

Changes required:

  1. Update Python CI pipelines to enable automatic reviews for management packages.
  2. Update all CI and release pipelines for management package supported languages to verify "Approved for first preview" status, which means package name is approved for preview release, and fail the release pipeline if not approved.

I assume API approval status check, which is done by architects after reviewing all APIs, is not in the scope of this change and feature request here is to check only for preview version basic approval like package name is valid etc.

@maririos @ronniegeraghty FYI....

RickWinter commented 1 year ago

@praveenkuttappan GO (and C++) need to have these approval checks as well.

cc: @sandeep-sen

maririos commented 1 year ago

Other things to think about:

RickWinter commented 1 year ago

@maririos, I don't agree with this being moved to the backlog. We've already had to retract mgmt plane packages from the go package manager because the name used was incorrect (and not approved).

maririos commented 1 year ago

@maririos, I don't agree with this being moved to the backlog. We've already had to retract mgmt plane packages from the go package manager because the name used was incorrect (and not approved).

The APIView issue is in backlog until the approval process is clearly defined and therefore the specific requirements for APIView. I've communicated your concern and urgency on having tooling be part of the approval process to our PM team (@kyle-patterson and @ronniegeraghty) who are leading this effort.

ladonnaq commented 1 year ago

@maririos @ronniegeraghty @praveenkuttappan I updated the description with the current summary of requirements. Let's use comments for discussions and then when we have agreement, hide the comment and update the description.

ladonnaq commented 1 year ago

@maririos @ronniegeraghty @praveenkuttappan

Discussion the arch board agreed upon process for naming approval for management plane

Let's review and identify what is needed to automate and include this in the release planner.

maririos commented 1 year ago

@ladonnaq

The release planner can generate the namespaces.

Could you provide the logic to make this happen?

ladonnaq commented 1 year ago

@ladonnaq

The release planner can generate the namespaces.

Could you provide the logic to make this happen?

@ronniegeraghty How would the generated namespace be determined from the PR?

Example: management (ARM) libraries

bolded and italicized text is namespace

maririos commented 12 months ago

Issue moved to the SDK Release epic so we can look into it there

maririos commented 7 months ago

@ronniegeraghty the current process is the service team needs to go to GH and create an issue following a template to provide the name. do you consider this could be something we should automate in the Release Planner? is there a template for the names to be proposed by automation too?

ronniegeraghty commented 7 months ago

YES! The Mgmt Plane Namespace Reviews are a great opportunity to automate a manual process. The names use the following patterns for each language:

The [ResourceProviderName] section should be replaced with the services branding name, usually the RP name. The casing of the section that replaces [ResourceProviderName] should follow the same casing it replaces.

Normally the service partner team submits a GH issues with their proposed names following the patterns. Then Arthur from the MGMT Plane team review the names with the API Spec and signs off on them. Then the names are shown to the whole architect team and the architects have a week to make any objections to the names. If there are no objections in the week then they are considered approved. If there are objections, the service partner team and architects work together to come up with a name that both sides agree on.

An automated version of this could look like the following: Service partner team comes to section in the release planner where they are asked to proposed namespaces for their MGMT Plane libraries. The names are auto-filled using the patterns and the services RP name from their spec, but the service team can edit them if that doesn't match their branding name. Once they confirm their proposed namespaces the architects are sent approvals to review in API View for the namespaces.

maririos commented 7 months ago

Awesome.

Once they confirm their proposed namespaces the architects are sent approvals to review in API View for the namespaces. Could you elaborate more on how from the proposed namespace confirmation on the Release Planner we go to APIViews with the namespace to approve?

An intermediate step could be: once they confirm their proposed namespaces the tool creates the GH issue with the proposal. And after that the process you said. Arthur reviews, and then the info is sent to architects.

ronniegeraghty commented 7 months ago

An intermediate step could be: once they confirm their proposed namespaces the tool creates the GH issue with the proposal. And after that the process you said. Arthur reviews, and then the info is sent to architects.

This could work. If the Release planner opens the GitHub Issue and Assigns it to Arthur. Then Arthur works with the partner team in the issue for the 1st phase of the review. Then once Arthur and the service team have come to an agreement, Arthur could add a specific label that triggers an email to be sent out to the architects and starts the 1 week count down. The architects we'll have to start putting their comments and objections in the issue comments and not in the email thread.

ladonnaq commented 6 months ago

@ronniegeraghty @maririos - Since Crystal will be on medical leave for 1H 2024, should I go ahead and schedule a meeting with you, Ronnie, and Jonathon so we can work out UI and back-end needed to implement this feature. I think it would be very helpful for our service teams.