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

write okify_regtests script, generalizing jwst and romancal versions #69

Merged

Conversation

zacharyburnett
Copy link
Collaborator

@zacharyburnett zacharyburnett commented Nov 13, 2024

resolves SCSB-183

follow-up to spacetelescope/jwst#8762

adds the okify_regtests script to the ci_watson package, generalized between the existing jwst and romancal versions:

This change would be coupled with removals of okify_regtests from both jwst and romancal:

Arguments

usage: okify_regtests [-h] [--dry-run] {jwst,roman} run-number

"okifies" a set of failing regression test results, by overwriting truth files on
Artifactory so that a set of failing regression test results becomes correct. Requires
JFrog CLI (https://jfrog.com/getcli/) configured with credentials (`jf login`) and write
access to the desired truth file repository (`jwst-pipeline`, `roman-pipeline`, etc.).

positional arguments:
  {jwst,roman}  Observatory to overwrite truth files for on Artifactory
  run-number    GitHub Actions job number of regression test run (see
                https://github.com/spacetelescope/RegressionTests/actions)

options:
  -h, --help    show this help message and exit
  --dry-run     do nothing (passes the `--dry-run` flag to JFrog CLI)

Usage Examples

okify_regtests jwst 956 --dry-run
okify_regtests roman 1317

@zacharyburnett zacharyburnett self-assigned this Nov 13, 2024
@zacharyburnett zacharyburnett changed the title write generalized okify_regtests script write okify_regtests script, generalizing jwst and romancal versions Nov 13, 2024
@zacharyburnett zacharyburnett changed the title write okify_regtests script, generalizing jwst and romancal versions write okify_regtests script, generalizing jwst and romancal versions Nov 13, 2024
@zacharyburnett
Copy link
Collaborator Author

@spacetelescope/jwst-pipeline-maintainers @spacetelescope/jwst-pipeline-developers @spacetelescope/romancal-maintainers thoughts on this consolidation?

@zacharyburnett
Copy link
Collaborator Author

added rglob from spacetelescope/jwst#8970

@zacharyburnett zacharyburnett marked this pull request as ready for review November 21, 2024 14:09
@WilliamJamieson
Copy link

Could you add basic documentation for this in the README or something like that?

@zacharyburnett
Copy link
Collaborator Author

zacharyburnett commented Nov 22, 2024

@melanieclarke as you use okify_regtests for JWST, would you mind testing this script in this branch when you get a chance, and let me know if you'd like any changes to the workflow (or if you find any bugs)?

@melanieclarke
Copy link

@zacharyburnett - will do, next time I have something to okify. Thanks for working on this!

@zacharyburnett zacharyburnett marked this pull request as draft November 22, 2024 15:55
@melanieclarke
Copy link

I tested okify_regtests from this branch for jwst for diffs from spacetelescope/jwst#8847, with: okify_regtests jwst 1076

It looks like all went well - it found the two diffs I expected and copied them to the right place in artifactory.

Copy link

@melanieclarke melanieclarke left a comment

Choose a reason for hiding this comment

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

LGTM, for jwst.

@zacharyburnett zacharyburnett marked this pull request as ready for review December 5, 2024 13:59
Copy link
Contributor

@braingram braingram left a comment

Choose a reason for hiding this comment

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

Tested with romancal (see spacetelescope/romancal#1513) with no issues. Feel free to merge (I don't have permissions to merge).

@zacharyburnett zacharyburnett merged commit d79db70 into spacetelescope:main Dec 9, 2024
10 checks passed
@zacharyburnett zacharyburnett deleted the scripts/okify_regtests branch December 9, 2024 15:19
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.

4 participants