-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[CI] if latest contexts have changed, retrieve the previous contexts' test cache and then use ./setup_test_cache u
to update
#1099
Conversation
c5ed0b1
to
d7dce5b
Compare
./setup_test_cache u
to update a previous existing cache./setup_test_cache u
to update
d7dce5b
to
393f1cb
Compare
tested at https://github.com/spacetelescope/stenv/actions/runs/12017528869/job/33500260337 by manually creating a
|
@spacetelescope/crds-maintainers does this look right? The log in the comment above (#1099 (comment)) show the latest context for both the HST and JWST commands in
However, the log says this throughout:
So did it actually update the test cache? |
- if: steps.lookup-cache.outputs.cache-hit != 'true' | ||
id: retrieve-previous-cache | ||
name: retrieve a previous combined CRDS cache | ||
uses: actions/cache/restore@v4 | ||
with: | ||
path: | | ||
${{ env.CRDS_PATH }} | ||
${{ env.CRDS_TESTING_CACHE }} | ||
key: ${{ steps.key.outputs.key }} | ||
restore-keys: | | ||
test-cache- |
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.
if a cache with the latest contexts does not exist, attempt to retrieve a previous cache that does exist (with the intent of running ./setup_test_cache /tmp u
on it later)
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.
I'm not really sure what is happening there. I know there are differences between the two contexts, but I feel like the test cache had nothing to update.
- if: steps.lookup-combined-cache.outputs.cache-hit != 'true' | ||
run: ./setup_test_cache ${{ env.CRDS_TEST_ROOT }} ${{ (steps.retrieve-hst-cache.outputs.cache-hit == 'true' || steps.retrieve-jwst-cache.outputs.cache-hit == 'true') && 'u' || 'c' }} | ||
- if: steps.lookup-combined-cache.outputs.cache-hit != 'true' | ||
- if: steps.lookup-cache.outputs.cache-hit != 'true' | ||
run: ./setup_test_cache ${{ env.CRDS_TEST_ROOT }} u |
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.
we don't ever need to run with c
here, only u
This should decrease the time it takes to make a new test cache for the latest contexts, when the contexts change. I need to test this against a context change to make sure that
u
behaves as expected EDIT: tested in #1099 (comment)This also removes duplication of caches by obviating the need for separate individual caches for each repository
Tasks
docs/
pageno-changelog-entry-needed
)changes/
:echo "changed something" > changes/<PR#>.<changetype>.rst
(see below for change types)news fragment change types...
changes/<PR#>.hst.rst
: HST reference fileschanges/<PR#>.jwst.rst
: JWST reference fileschanges/<PR#>.roman.rst
: Roman reference fileschanges/<PR#>.doc.rst
: documentation changechanges/<PR#>.testing.rst
: change to tests or test automationchanges/<PR#>.general.rst
: infrastructure or miscellaneous change