Migrate Python checks to CI Gate - #11522
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: df233d93-3288-4a30-a629-d9e53a234021
|
All changed packages have been documented.
Show changes
|
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: df233d93-3288-4a30-a629-d9e53a234021
|
You can try these changes here
|
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: df233d93-3288-4a30-a629-d9e53a234021
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: df233d93-3288-4a30-a629-d9e53a234021
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: df233d93-3288-4a30-a629-d9e53a234021
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: df233d93-3288-4a30-a629-d9e53a234021
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: df233d93-3288-4a30-a629-d9e53a234021
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: df233d93-3288-4a30-a629-d9e53a234021
Install the Python package dev requirements in each CI Gate job so tox-based test, lint, docs, and typecheck commands can run after restoring generated artifacts. Also enable Git long paths before the Windows checkout to avoid failures on long test-data paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: df233d93-3288-4a30-a629-d9e53a234021
Pre-build generated package wheels in the build job and upload them with generated artifacts. Run downstream jobs against restored artifacts with package install plus a fail-fast Python prepare step instead of rebuilding the package in each job. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: df233d93-3288-4a30-a629-d9e53a234021
Rename the Linux build job and make the Windows Python CI job build, regenerate, and test independently without restoring Linux artifacts. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: df233d93-3288-4a30-a629-d9e53a234021
Allow the Python generated artifact upload to include the hidden .wheels directory so downstream Linux jobs install pre-built wheels instead of falling back to source builds. Add concise comments documenting the CI-specific npm and Python prepare steps. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: df233d93-3288-4a30-a629-d9e53a234021
Download generated Python artifacts into packages/http-client-python so downstream jobs find tests/generated, tests/.wheels, dist, and generator at the paths expected by the package scripts. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: df233d93-3288-4a30-a629-d9e53a234021
Add direct dev dependencies for eslint and @eslint/js because the standalone http-client-python lint config imports @eslint/js in CI. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: df233d93-3288-4a30-a629-d9e53a234021
commit: |
Use a package-local Prettier config for standalone http-client-python formatting so CI does not require the monorepo TypeSpec Prettier plugin build output. Avoid importing @eslint/js from the standalone ESLint config and add an internal Chronus entry for the Python CI Gate migration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: df233d93-3288-4a30-a629-d9e53a234021
Build the TypeSpec Prettier plugin only in the Python Lint & Format job before running package format checks, instead of carrying a separate package-local Prettier config. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: df233d93-3288-4a30-a629-d9e53a234021
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Suppressed comments (5)
.github/workflows/ci-python.yml:88
npm install --package-lock=falsedisables the checked-in lockfile, making CI dependency resolution non-deterministic and potentially masking/failing on transient dependency updates. Prefernpm ci --ignore-scriptsfor reproducible CI installs without lockfile rewrites.
npm install --ignore-scripts --package-lock=false
.github/workflows/ci-python.yml:122
npm install --package-lock=falsedisables the checked-in lockfile, making CI dependency resolution non-deterministic. Prefernpm ci --ignore-scriptsfor reproducible installs in CI.
npm install --ignore-scripts --package-lock=false
.github/workflows/ci-python.yml:201
npm install --package-lock=falsedisables the checked-in lockfile, making CI dependency resolution non-deterministic. Prefernpm ci --ignore-scriptsfor reproducible installs in CI.
npm install --ignore-scripts --package-lock=false
.github/workflows/ci-python.yml:231
npm install --package-lock=falsedisables the checked-in lockfile, making CI dependency resolution non-deterministic. Prefernpm ci --ignore-scriptsfor reproducible installs in CI (works on Windows too).
npm install --ignore-scripts --package-lock=false
.github/workflows/ci-python.yml:156
npm install --package-lock=falsedisables the checked-in lockfile, making CI dependency resolution non-deterministic. Prefernpm ci --ignore-scriptsfor reproducible installs in CI.
npm install --ignore-scripts --package-lock=false
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: df233d93-3288-4a30-a629-d9e53a234021
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
.github/workflows/ci-python.yml:52
actions/upload-artifactis referenced via a floating major tag (@v7). Pin it to a full commit SHA (as done in other workflows) to avoid supply-chain risk and unexpected behavior changes.
- name: Upload generated artifacts
uses: actions/upload-artifact@v7
.github/workflows/ci-python.yml:77
actions/download-artifactis referenced via a floating major tag (@v8). Pin it to a full commit SHA (as done in other workflows) to avoid supply-chain risk and unexpected behavior changes. This same change should be applied to the otherdownload-artifactsteps in this workflow.
- name: Download generated artifacts
uses: actions/download-artifact@v8
|
Timothee Guerin (@timotheeguerin) Is there other concern for this PR? |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: df233d93-3288-4a30-a629-d9e53a234021
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (17)
.github/workflows/ci-python.yml:18
- This workflow uses unpinned GitHub Actions versions (e.g.
actions/checkout@v7,actions/setup-python@v7). Elsewhere in this repo, actions are pinned to a full commit SHA to reduce supply-chain risk and make builds reproducible. Please pin these actions here as well.
- uses: actions/checkout@v7
- uses: ./.github/actions/setup
- uses: actions/setup-python@v7
.github/workflows/ci-python.yml:53
actions/upload-artifactshould be pinned to a commit SHA (consistent with other workflows in this repo) to avoid supply-chain risk from mutable tags.
- name: Upload generated artifacts
uses: actions/upload-artifact@v7
with:
.github/workflows/ci-python.yml:72
- This job uses unpinned GitHub Actions versions (
actions/checkout@v7,actions/setup-python@v7). Pinning to a full commit SHA is the established pattern in this repo and reduces supply-chain risk.
- uses: actions/checkout@v7
- uses: ./.github/actions/setup
- uses: actions/setup-python@v7
.github/workflows/ci-python.yml:80
actions/download-artifactshould be pinned to a commit SHA (as in other workflows) to avoid supply-chain risk from mutable tags.
- name: Download generated artifacts
uses: actions/download-artifact@v8
with:
name: python-generated
path: packages/http-client-python
.github/workflows/ci-python.yml:106
- This job uses unpinned GitHub Actions versions (
actions/checkout@v7,actions/setup-python@v7). Pin to commit SHAs for consistency and to reduce supply-chain risk.
- uses: actions/checkout@v7
- uses: ./.github/actions/setup
- uses: actions/setup-python@v7
.github/workflows/ci-python.yml:114
actions/download-artifactshould be pinned to a commit SHA (as in other workflows) to avoid supply-chain risk from mutable tags.
- name: Download generated artifacts
uses: actions/download-artifact@v8
with:
name: python-generated
path: packages/http-client-python
.github/workflows/ci-python.yml:140
- This job uses unpinned GitHub Actions versions (
actions/checkout@v7,actions/setup-python@v7). Pin to commit SHAs for consistency and to reduce supply-chain risk.
- uses: actions/checkout@v7
- uses: ./.github/actions/setup
- uses: actions/setup-python@v7
.github/workflows/ci-python.yml:148
actions/download-artifactshould be pinned to a commit SHA (as in other workflows) to avoid supply-chain risk from mutable tags.
- name: Download generated artifacts
uses: actions/download-artifact@v8
with:
name: python-generated
path: packages/http-client-python
.github/workflows/ci-python.yml:185
- This job uses unpinned GitHub Actions versions (
actions/checkout@v7,actions/setup-python@v7). Pin to commit SHAs for consistency and to reduce supply-chain risk.
- uses: actions/checkout@v7
- uses: ./.github/actions/setup
- uses: actions/setup-python@v7
.github/workflows/ci-python.yml:193
actions/download-artifactshould be pinned to a commit SHA (as in other workflows) to avoid supply-chain risk from mutable tags.
- name: Download generated artifacts
uses: actions/download-artifact@v8
with:
name: python-generated
path: packages/http-client-python
.github/workflows/ci-python.yml:221
- This job uses unpinned GitHub Actions versions (
actions/checkout@v7,actions/setup-python@v7). Pin to commit SHAs for consistency and to reduce supply-chain risk.
- uses: actions/checkout@v7
- uses: ./.github/actions/setup
- uses: actions/setup-python@v7
.github/workflows/ci-python.yml:28
npm install --package-lock=falsebypasses the committedpackage-lock.json, making CI installs non-deterministic and potentially flaky. If the goal is to avoid running lifecycle scripts while still using the lockfile,npm ci --ignore-scriptsis deterministic and does not rewrite the lockfile.
npm install --ignore-scripts --package-lock=false
.github/workflows/ci-python.yml:88
npm install --package-lock=falsebypasses the committedpackage-lock.json, making CI installs non-deterministic. Prefernpm ci --ignore-scriptsto use the lockfile without executing lifecycle scripts.
npm install --ignore-scripts --package-lock=false
.github/workflows/ci-python.yml:122
npm install --package-lock=falsebypasses the committed lockfile, which can make CI results depend on registry state. Prefernpm ci --ignore-scriptsfor deterministic installs.
npm install --ignore-scripts --package-lock=false
.github/workflows/ci-python.yml:156
npm install --package-lock=falsebypasses the committed lockfile. Prefernpm ci --ignore-scriptsfor deterministic CI installs without lifecycle scripts.
npm install --ignore-scripts --package-lock=false
.github/workflows/ci-python.yml:201
npm install --package-lock=falsebypasses the committed lockfile. Prefernpm ci --ignore-scriptsfor deterministic CI installs without lifecycle scripts.
npm install --ignore-scripts --package-lock=false
.github/workflows/ci-python.yml:231
npm install --package-lock=falsebypasses the committed lockfile, which can make CI nondeterministic. Prefernpm ci --ignore-scriptsfor deterministic installs.
npm install --ignore-scripts --package-lock=false
There was a problem hiding this comment.
🟡 Changes recommended
The new ci-python.yml workflow uses floating GitHub Action version tags instead of the repo’s pinned-SHA convention, which is a supply-chain/reproducibility risk.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
.github/workflows/ci-python.yml:52
- Pin
actions/upload-artifactto a commit SHA (repo convention) instead of a floating@v7tag to keep the workflow reproducible and reduce supply-chain risk.
uses: actions/upload-artifact@v7
.github/workflows/ci-python.yml:77
- Pin
actions/download-artifactto a commit SHA (repo convention) instead of a floating@v8tag to keep the workflow reproducible and reduce supply-chain risk. Same applies to the other download steps in this file.
uses: actions/download-artifact@v8
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Lite
For #11308
This PR update http-client-pytthon CI to replace ADO pipeline with github action and CI cost time is reduced from 60 minutes to < 40 minutes.
NOTE: I estimate there are still optimization space about 10 minutes but it needs some code change inner python script so will make another PR to do it.