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

vendorize and fix workflows #1619

Merged
merged 1 commit into from
Feb 14, 2025
Merged

Conversation

braingram
Copy link
Collaborator

@braingram braingram commented Feb 13, 2025

The cache workflows in webbpsf are out of date and use an exact pinned version of the actions/cache/save workflow (v4.0.2) which is no longer supported:
https://github.com/actions/cache/releases/tag/v4.2.0

The corresponding workflow in stpsf (the renamed version which now lives in a different repository):
https://github.com/spacetelescope/stpsf/blob/develop/.github/workflows/download_data.yml
contains other updates which make it incompatible with romancal usage.

This PR vendorizes the non-working workflow from webbpsf and fixes it (updates the workflow version).

This PR will not fix the CI until:

  • it's merged
  • a new webbpsf data cache is downloaded

Tasks

  • request a review from someone specific, to avoid making the maintainers review every PR
  • add a build milestone, i.e. 24Q4_B15 (use the latest build if not sure)
  • Does this PR change user-facing code / API? (if not, label with no-changelog-entry-needed)
    • write news fragment(s) in changes/: echo "changed something" > changes/<PR#>.<changetype>.rst (see below for change types)
    • update or add relevant tests
    • update relevant docstrings and / or docs/ page
    • start a regression test and include a link to the running job (click here for instructions)
      • Do truth files need to be updated ("okified")?
        • after the reviewer has approved these changes, run okify_regtests to update the truth files
  • if a JIRA ticket exists, make sure it is resolved properly
news fragment change types...
  • changes/<PR#>.general.rst: infrastructure or miscellaneous change
  • changes/<PR#>.docs.rst
  • changes/<PR#>.stpipe.rst
  • changes/<PR#>.associations.rst
  • changes/<PR#>.scripts.rst
  • changes/<PR#>.mosaic_pipeline.rst
  • changes/<PR#>.patch_match.rst

steps

  • changes/<PR#>.dq_init.rst
  • changes/<PR#>.saturation.rst
  • changes/<PR#>.refpix.rst
  • changes/<PR#>.linearity.rst
  • changes/<PR#>.dark_current.rst
  • changes/<PR#>.jump_detection.rst
  • changes/<PR#>.ramp_fitting.rst
  • changes/<PR#>.assign_wcs.rst
  • changes/<PR#>.flatfield.rst
  • changes/<PR#>.photom.rst
  • changes/<PR#>.flux.rst
  • changes/<PR#>.source_detection.rst
  • changes/<PR#>.tweakreg.rst
  • changes/<PR#>.skymatch.rst
  • changes/<PR#>.outlier_detection.rst
  • changes/<PR#>.resample.rst
  • changes/<PR#>.source_catalog.rst

@braingram braingram marked this pull request as ready for review February 13, 2025 22:20
@braingram braingram requested a review from a team as a code owner February 13, 2025 22:20
Copy link
Collaborator

@schlafly schlafly left a comment

Choose a reason for hiding this comment

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

I'm happy to approve, but if you and Zach plan a fix to the upstream workflow tomorrow, that's great too.

@braingram
Copy link
Collaborator Author

Thanks! @schlafly I can't merge this with the failing branch protections. Would you merge it and I'll regenerate the cache? Its unclear to me what the upstream fix needs to be. It looks like with the renaming of webbpsf to stpsf they changed the data download and the contents so I'm not at the moment certain if the new upstream files are suitable (this PR still uses the old files).

@schlafly schlafly merged commit 4427b8f into spacetelescope:main Feb 14, 2025
28 of 31 checks passed
@braingram braingram deleted the cache_fix branch February 14, 2025 15:06
@schlafly
Copy link
Collaborator

Merged. Okay, then we'll prioritize the webbpsf <-> stpsf migration.

@braingram
Copy link
Collaborator Author

Thanks! I opened an issue with a link to the migration documentation:
#1622
shared with me by @BradleySappington

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.

2 participants