Skip to content
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

[v24.1.x] gha: fix pip install on python actions #23766

Merged
merged 1 commit into from
Oct 14, 2024

Conversation

clee
Copy link
Contributor

@clee clee commented Oct 12, 2024

Backport of PR #23715

@clee clee requested a review from a team as a code owner October 12, 2024 02:05
@clee clee requested review from ivotron and removed request for a team October 12, 2024 02:05
@ivotron ivotron changed the title gha: fix pip install on python actions [v24.1.x] gha: fix pip install on python actions Oct 12, 2024
@clee clee force-pushed the backport-v24-1-x/fix-python-action branch from 4045a41 to 269927d Compare October 12, 2024 15:07
@clee clee requested a review from ivotron October 12, 2024 15:07
Copy link
Member

@andrewhsu andrewhsu left a comment

Choose a reason for hiding this comment

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

LGTM

just comment on alternative approach for future, this PR looks good as backport

@@ -23,7 +23,12 @@ jobs:
- name: install dependencies
env:
SCRIPT_DIR: ${{ github.workspace }}/.github/workflows/scripts
run: pip install -r ${SCRIPT_DIR}/requirements.txt
run: |
python3 -mvenv /tmp/venv/jira
Copy link
Member

Choose a reason for hiding this comment

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

fyi instead of manually creating a venv, could also use the setup-python action which updates the PATH with isolated python interpreter and save from having to source the activate script, e.g.:

      - uses: actions/setup-python@v5
        with:
          python-version: '3.10'

Copy link
Member

Choose a reason for hiding this comment

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

oops, this comment should have gone into the lint-python.yml workflow file because there already is a setup-python above

@@ -22,7 +22,10 @@ jobs:
uses: actions/checkout@v4

- name: Install yapf
run: pip install yapf==0.40.1
run: |
python3 -mvenv /tmp/venv/yapf
Copy link
Member

Choose a reason for hiding this comment

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

fyi instead of manually creating a venv, could also use the setup-python action which updates the PATH with isolated python interpreter and save from having to source the activate script, e.g.:

      - uses: actions/setup-python@v5
        with:
          python-version: '3.10'

@clee clee merged commit 488947b into v24.1.x Oct 14, 2024
10 checks passed
@clee clee deleted the backport-v24-1-x/fix-python-action branch October 14, 2024 16:33
@BenPope BenPope added this to the v24.1.18 milestone Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants