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

DM-46034: Fix implementation of use-cache #5

Merged
merged 2 commits into from
Sep 5, 2024
Merged

DM-46034: Fix implementation of use-cache #5

merged 2 commits into from
Sep 5, 2024

Conversation

rra
Copy link
Member

@rra rra commented Sep 4, 2024

The way the fromJSON trick to force a boolean argument to a boolean was used here appears to have stopped working (or perhaps hadn't worked before?) based on GitHub Actions output for safir and (in the case of run-tox, which has the same construction) Gafaelfawr. Make the construction match what https://github.com/lsst-sqre/build-and-publish-to-pypi does which seems to work better.

The `fromJSON` trick to force a boolean argument to a boolean
appears to not work based on GitHub Actions output for safir and
(in the case of run-tox, which has the same construction) Gafaelfawr.
Compare use-cache against a string 'true' instead, which is reported
to work correctly.
@rra
Copy link
Member Author

rra commented Sep 4, 2024

Tested with Safir. Compare workflow runs https://github.com/lsst-sqre/safir/actions/runs/10692720827/job/29641658832 (fixed) and https://github.com/lsst-sqre/safir/actions/runs/10667368211/job/29641353960 and notice how the latter still restored the cache, but the former did not.

See actions/runner#2238 for more details.

@rra
Copy link
Member Author

rra commented Sep 4, 2024

New test using the same construction as build-and-publish-to-pypi: https://github.com/lsst-sqre/safir/actions/runs/10704089751/job/29676203060

Booleans were working correctly with the build-and-publish-to-pypi
action, so use the same construction that was used there. Perhaps
the interpolation using ${{}} or the lack of a comparison was
creating the problem.
@rra rra merged commit 6dc0d6c into main Sep 5, 2024
2 checks passed
@rra rra deleted the tickets/DM-46034 branch September 5, 2024 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants