Skip to content

Python POC for GitHub-based API Reviews#47203

Open
tjprescott wants to merge 30 commits into
mainfrom
GenerateAPITextScript
Open

Python POC for GitHub-based API Reviews#47203
tjprescott wants to merge 30 commits into
mainfrom
GenerateAPITextScript

Conversation

@tjprescott
Copy link
Copy Markdown
Member

@tjprescott tjprescott commented May 28, 2026

Summary

Adds tooling for GitHub-native API reviews for the Python SDK, enabling API surface tracking and review via GitHub PRs and CI. Based on this proposal: Proposal: API Reviews via GitHub (azure-sdk-tools#15789).

What's Included

1. CI Consistency Check (.github/workflows/api-consistency.yml)

A GitHub Actions workflow that enforces API.md consistency on every PR that touches sdk/**:

  1. find_affected.js — diffs the PR against the base branch to find changed packages
  2. regenerate.js — runs azpysdk apistub --md --extract-metadata for each affected package
  3. find_mismatches.js — compares the regenerated API.md to the committed version and validates select metadata fields
  4. Fails with actionable instructions if drift is detected

2. API Review PR Creator (create_api_review_pr.js)

A CLI tool to create dedicated API review PRs:

node scripts/api_md_workflow/create_api_review_pr.js \
  --package-name azure-foo-bar \
  --base azure-foo-bar_1.0.0 \
  --target owner:branch
  • Generates API.md for both the base tag and target branch
  • Pushes two branches and opens a draft PR where the diff is the API diff
  • Reviewers use GitHub's native review tools (comments, suggestions, approvals)

3. Language Adapter Pattern (scripts/api_md_workflow/)

Core orchestration is language-agnostic. A per-repo adapter (adapters/python.js) implements the language-specific logic (isPackageDir, findPackageDir, readVersion, generateApiForPackage). This allows the same framework to be ported to other Azure SDK language repos.

4. Shared Utility Layer (.github/shared/)

Common ESM modules for subprocess execution, logging, path manipulation, and GitHub API interactions — shared across workflow scripts.

Examples

Auto Reviews

PR Review

@tjprescott tjprescott requested a review from kashifkhan May 28, 2026 16:55
@tjprescott tjprescott requested a review from scbedd as a code owner May 28, 2026 16:55
Copilot AI review requested due to automatic review settings May 28, 2026 16:55
@tjprescott tjprescott requested a review from danieljurek as a code owner May 28, 2026 16:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new repo script (scripts/generate_api_text.py) to generate a package-level API.md by building a wheel, running apiview-stub-generator (apistub) to produce the token JSON, and converting that JSON to markdown via Export-APIViewMarkdown.ps1. This is repo tooling intended to streamline API review artifact generation.

Changes:

  • Introduces scripts/generate_api_text.py to locate a package under sdk/*/, build a wheel, run apistub, and emit API.md into the package directory.
  • Adds logic to install eng/apiview_reqs.txt and (optionally) upgrade apiview-stub-generator from the Azure SDK package index.
  • Uses eng/common/scripts/Export-APIViewMarkdown.ps1 to convert the generated *_python.json token file into markdown.

Comment thread scripts/generate_api_text.py Outdated
Comment thread scripts/generate_api_text.py Outdated
Comment thread scripts/generate_api_text.py Outdated
Comment thread scripts/generate_api_text.py Outdated
Comment thread scripts/generate_api_text.py Outdated
Comment thread scripts/generate_api_text.py Outdated
Comment thread scripts/generate_api_text.py Outdated
@tjprescott tjprescott force-pushed the GenerateAPITextScript branch 3 times, most recently from 7c45b15 to b9dc383 Compare May 29, 2026 19:55
@tjprescott tjprescott closed this May 29, 2026
@tjprescott tjprescott reopened this May 29, 2026
@tjprescott tjprescott changed the title Add generate_api_text.py script Python POC for GitHub-based API Reviews Jun 1, 2026
@tjprescott tjprescott force-pushed the GenerateAPITextScript branch from f320bdb to e4b2876 Compare June 2, 2026 17:59
@tjprescott tjprescott requested a review from a team as a code owner June 2, 2026 17:59
Comment thread .github/workflows/consistency.yml Outdated
Comment thread .github/workflows/consistency.yml Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 9 comments.

Comments suppressed due to low confidence (1)

eng/tools/azure-sdk-tools/azpysdk/apistub.py:158

  • --dest-dir now writes outputs directly into the destination directory for every targeted package. When multiple packages are targeted in a single azpysdk apistub invocation, later packages can overwrite earlier ones (e.g., generic api.md / API.metadata.yml), causing incorrect/stale results.
            dest_dir = getattr(args, "dest_dir", None)
            if dest_dir:
                out_token_path = os.path.abspath(dest_dir)
                os.makedirs(out_token_path, exist_ok=True)
            else:
                out_token_path = os.path.abspath(staging_directory)

Comment thread scripts/api_md_workflow/regenerate.js
Comment thread scripts/api_md_workflow/README.md Outdated
Comment thread scripts/api_md_workflow/README.md Outdated
Comment thread .github/workflows/src/api-md-consistency/api-md-consistency.js
Comment thread scripts/api_md_workflow/find_mismatches.js
Comment thread sdk/template/azure-template/API.md Outdated
Comment thread sdk/template/azure-template/README.md Outdated
Comment thread .github/shared/src/cache.js
Comment thread scripts/api_md_workflow/adapters/python.js
@tjprescott tjprescott force-pushed the GenerateAPITextScript branch from 580208a to 4b6f18e Compare June 3, 2026 21:27
@tjprescott tjprescott force-pushed the GenerateAPITextScript branch from 4b6f18e to 9d8137c Compare June 3, 2026 21:50
@tjprescott tjprescott force-pushed the GenerateAPITextScript branch 3 times, most recently from 6d81d65 to 818a5ee Compare June 5, 2026 21:08
@tjprescott tjprescott requested a review from Copilot June 5, 2026 22:27
@tjprescott tjprescott force-pushed the GenerateAPITextScript branch from 818a5ee to 778edda Compare June 5, 2026 22:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (1)

scripts/api_md_workflow/README.md:112

  • Same issue later in the README: the printed regeneration command uses --dest-dir . and omits --install-deps, which won’t reproduce the CI behavior and will write files to the wrong directory when run from the repo root. This should point --dest-dir at the package directory and include --install-deps.
4. `regenerate.js` rebuilds `api.md` for those packages.
5. `find_mismatches.js` records any `api.md` drift, including missing or untracked `api.md` files.
6. If drift is found, the workflow fails and prints the affected packages plus the `azpysdk apistub --md --extract-metadata <package-name> --dest-dir .` command to regenerate each `api.md` file locally from the repository root.

Comment thread .github/workflows/src/api-md-consistency/api-md-consistency.js
Comment thread scripts/api_md_workflow/README.md
Comment thread .github/skills/generate-api-markdown/SKILL.md
Comment thread eng/tools/azure-sdk-tools/tests/test_apistub.py Outdated
Comment thread .github/workflows/api-consistency.yml
Comment thread scripts/api_md_workflow/find_mismatches.js
@tjprescott tjprescott force-pushed the GenerateAPITextScript branch from 778edda to a82a7ee Compare June 5, 2026 22:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 4 comments.

Comment on lines +43 to +48
const packageDir = apiFile.replace(/\/(api\.md|api\.metadata\.yml)$/, "");
const packageName = path.basename(packageDir);
lines.push(`- ${packageDir}`);
lines.push(` API file: ${apiFile}`);
lines.push(` Regenerate: azpysdk apistub --md --extract-metadata ${packageName} --dest-dir .`);
}
Comment on lines +21 to +26
- Detects affected package directories from the PR diff.
- Regenerates `api.md` for those packages.
- Fails if the generated files differ from the committed files.
- Fails if an affected package does not have a committed `api.md`.
- Prints the mismatched or missing packages and the `azpysdk apistub --md --extract-metadata <package-name> --dest-dir .` command needed to regenerate each `api.md` file from the repository root.

Comment on lines +106 to +111
1. A PR changes files under `sdk/**`.
2. `consistency.yml` runs.
3. `find_affected.js` determines which packages were touched.
4. `regenerate.js` rebuilds `api.md` for those packages.
5. `find_mismatches.js` records any `api.md` drift, including missing or untracked `api.md` files.
6. If drift is found, the workflow fails and prints the affected packages plus the `azpysdk apistub --md --extract-metadata <package-name> --dest-dir .` command to regenerate each `api.md` file locally from the repository root.
Comment on lines +63 to +70
function isPackageDir(repoRoot, packageDirRelative) {
const candidate = path.join(repoRoot, packageDirRelative);
if (!fs.existsSync(candidate) || !fs.statSync(candidate).isDirectory()) {
return false;
}

return fs.existsSync(path.join(candidate, "pyproject.toml")) || fs.existsSync(path.join(candidate, "setup.py"));
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we will need to either add a package-lock.json file, or add the necessary deps to another package.json already in the repo

const path = require("path");
const { spawnSync } = require("child_process");

function runNode(scriptRelativePath, workspace, core) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

use a helper from exec.js, or we can add another helper if needed. we have a helper to run an arbitary file, or a helper to run "npm", but not a helper to run "node" itself (though we could add one).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants