-
Notifications
You must be signed in to change notification settings - Fork 2
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
Refactor GHA to use composite action instead of workflow #17
Refactor GHA to use composite action instead of workflow #17
Conversation
86f41a0
to
fbe1a0a
Compare
Codecov Report
@@ Coverage Diff @@
## master #17 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 4 4
Lines 439 439
=========================================
Hits 439 439 |
ab3a7ea
to
c799892
Compare
@@ -15,6 +15,8 @@ | |||
\.lintr$ | |||
\.travis\.yml$ | |||
^.*\.Rproj$ | |||
^.*\.db$ |
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.
This is to prevent the PTAXSIM database file from being copied into the build directory during install.
.github/workflows/R-CMD-check.yaml
Outdated
- name: Prepare PTAXSIM database | ||
uses: ./.github/actions/prepare-ptaxsim | ||
with: | ||
ASSUMED_ROLE: ${{ secrets.AWS_IAM_ROLE_TO_ASSUME_ARN }} |
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.
This is a hacky way to pass a secret to a composite action. Needed until this issue actions/toolkit#1168 is resolved. It still properly masks the secret in the output logs.
@jeancochrane or @wrridgeway, can one of you give this a quick gander before I yolo this into master? |
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.
Neat stuff.
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.
Nice refactor!
- name: Save database cache | ||
uses: actions/cache/save@v3 | ||
id: save_db | ||
if: steps.restore_db_cache.outputs.cache-hit != 'true' | ||
with: | ||
path: ptaxsim.db.bz2 | ||
key: ${{ format('{0}-{1}', env.PTAXSIM_VERSION, hashFiles('DESCRIPTION')) }} | ||
enableCrossOsArchive: true |
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.
[Question, non-blocking] I'm not quite an expert on GA caching, what's the benefit for this action of splitting save
and restore
into two steps instead of using the combined actions/cache
action that handles both?
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.
No benefit at all, it's just an artifact of the complexity from pre-refactor. Fixed by 938f629!
This PR simplifies PTAXSIM's GitHub Actions workflows. It replaces the composable workflow to download and extract PTAXSIM with a single composite action. It also replaces long-lived AWS credentials with an OIDC assumed role.
Closes #15 and #16.