-
Notifications
You must be signed in to change notification settings - Fork 860
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
Use setup python action #9123
Use setup python action #9123
Conversation
This should only take a few extra seconds, and might help catch problems at PR time instead of at deploy time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks okay, none of my comments are show stoppers...
npm --production=false ci | ||
mkdir -p ./test/results | ||
- name: setup Python | ||
uses: actions/setup-python@v5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a Github supported version? (v5)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, 5.3.0 is the current version of actions/setup-python
and v5
should pick that because semver. Also, Renovate will replace @v5
with @specific-hash
once this gets merged.
uses: actions/setup-python@v5 | ||
with: | ||
python-version: '^3.5' | ||
- run: pip install s3cmd==2.3.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Longer term question: should we switch over to using AWS's python SDK and/or CLI for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I agree with that question. I'd expand it a bit:
- Is
s3cmd
the best way to deployscratch-www
? (This is my version of your question) - If so, is
s3cmd
the only thing this workflow uses Python for? - If so, should we get/use
s3cmd
some other way, like through one of thes3cmd
actions?
It could be that our current setup is the best option, but it's been long enough since we set this up that I think it deserves some investigation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding the questions above - I don't have context on your previous considerations (and why s3cmd was chosen specifically) and I'm just starting to get acquainted with the deploy options;
from initial research though - AWS CLI does look like a simpler option that will look mostly the same in terms of workflow implementation - with the + that we won't have a python dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Thank you for fixing this!
Resolves:
Resolves the current build failure preventing both staging and production deploys.
Changes:
Before:
pip
that is compatible with Python 3.5, thenpip
to installs3cmd==2.3.0
After:
actions/setup-python
to ensure availability of a version of Python that is compatible with v3.5, thenpip
to installs3cmd==2.3.0
Since these commands are relatively quick, I moved them earlier in the workflow. Also, while they're only necessary in the case of a deploy, running them even when not deploying means that if a PR breaks them, we'll know about it before we try to deploy.
Test Coverage:
Tested by manually running the GHA workflow on the branch. See also the checks on this PR.