Python POC for GitHub-based API Reviews (Phase 3)#47479
Conversation
|
|
||
| ## Goal | ||
|
|
||
| When an API review PR is marked approved in GitHub, record the approved package version and API hash on the package's Azure DevOps work item. During the Python release process, extend the existing APIVIew gating flow so it first checks whether the release artifact's exact version and API hash were approved in the work item. If that new check passes, the package is approved. If it fails, fall back to the existing APIView approval check. |
There was a problem hiding this comment.
Can ADO work item contains link to pending review PR? From an agentic perspective: if agent says API review is not approved, it should also show a link to what's not approved. Can API review PR be queried using package name and version or is this something recorded in work item to help the user?
| ## Terms | ||
|
|
||
| - **Review PR:** The GitHub PR created by `scripts/api_md_workflow/create_api_review_pr.py` for an `api.md` diff. The PR title starts with `[API Review]` and the PR body contains hidden `api-md-review-sync` metadata. | ||
| - **API hash:** The normalized API markdown SHA-256 stored in `api.metadata.yml` as `apiMdSha256`. This hash is produced by `eng/scripts/Extract-APIViewMetadata-Python.ps1` after removing parser metadata and normalizing trailing whitespace and line endings. |
There was a problem hiding this comment.
How about documentation lines in the review PR? or is it not included in api.md?
| - **Review PR:** The GitHub PR created by `scripts/api_md_workflow/create_api_review_pr.py` for an `api.md` diff. The PR title starts with `[API Review]` and the PR body contains hidden `api-md-review-sync` metadata. | ||
| - **API hash:** The normalized API markdown SHA-256 stored in `api.metadata.yml` as `apiMdSha256`. This hash is produced by `eng/scripts/Extract-APIViewMetadata-Python.ps1` after removing parser metadata and normalizing trailing whitespace and line endings. | ||
| - **Approved API tuple:** The package name, major.minor release line, full package version, API hash, review PR number, review commit, and approval state recorded on the Azure DevOps package work item. | ||
| - **Major.minor release line:** The `MAJOR.MINOR` portion of the package version. Current ADO package work items are scoped to this release line, so one work item can contain approvals for multiple patch and beta versions such as `1.2.0b1`, `1.2.0b2`, and `1.2.0`. |
There was a problem hiding this comment.
Is patch version expected to have API change? Any API surface change should be a minor version change. right?
|
|
||
| Add two automation paths: | ||
|
|
||
| 1. **Approval recorder.** Runs as a GitHub Actions workflow when a GitHub review is submitted, dismissed, or when an approved API review PR receives a new commit. It looks up the package work item with `azsdk-cli package get-work-item`, then writes, revokes, or supersedes the approved version/hash record. |
There was a problem hiding this comment.
From a security perspective: We should ensure that only reviews created with write permission in the repo are considered by the action to avoid updating the apiview status for a random review created b external users.
| "majorMinorVersion": "1.2", | ||
| "apiApprovals": [ | ||
| { | ||
| "version": "1.2.0b1", |
There was a problem hiding this comment.
Do we really need API review details per version or is it good enough to store one set per major.minor version?
| } | ||
| ``` | ||
|
|
||
| An active release approval is the single entry for the release candidate's full `version` in the matching package and major.minor work item. The work-item side of the OR check passes only when that entry's `apiMdSha256` matches the release artifact and `state` is `approved`. If the entry is missing, has a different hash, or has any other state, the work-item check does not pass and the APIVIew gating flow falls back to APIView. |
There was a problem hiding this comment.
Something seems missing in this sentence.
The work-item side of the OR check passes only when that entry's apiMdSha256matches the release artifact andstateisapproved``
| azsdk-cli package get-work-item --package-name <package> --language python --major-minor-version <major.minor> --output json | ||
| azsdk-cli package approve-api --work-item-id <id> --version <version> --api-hash <hash> --review-pr <url> --review-commit <sha> --approved-by <github-login> | ||
| azsdk-cli package revoke-api-approval --work-item-id <id> --version <version> --api-hash <hash> | ||
| azsdk-cli package check-api-approval --work-item-id <id> --version <version> --api-hash <hash> |
There was a problem hiding this comment.
Can check-api-approval cli be changed to take package name instead of work item id as param? This will help us expose this CLI/MCP to agent to help users check apireview approval for a package and version. If --api-hash is provided then checks for exact hash presence in ADO. Otherwise return the status of latest version.
|
|
||
| ```yaml | ||
| on: | ||
| pull_request_review: |
There was a problem hiding this comment.
This will need to be changed into a two stage GitHub action for security reason. First one performs a check and record status (this has no write permission to ADO) and second one that depends on first one runs ADO update. Let's discuss more when we meet.
2 participants
Proposal for the mechanism of recording API approvals in ADO work items and updating pipeline release checks to look at ADO work items instead of APIView.