-
Notifications
You must be signed in to change notification settings - Fork 174
JP-3773: Store logs from calibration pipeline in datamodel
#9211
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9211 +/- ##
==========================================
+ Coverage 73.65% 73.68% +0.02%
==========================================
Files 368 369 +1
Lines 36374 36406 +32
==========================================
+ Hits 26792 26824 +32
Misses 9582 9582 ☔ View full report in Codecov by Sentry. |
One issue that popped up in my testing - I ran some files locally, replacing the scrubbing behavior with a prepended scrub tag to see what's getting removed. The only scrub case I saw in testing was the removal of statements recording which reference files are being used, due to my user ID being present in the directory tree of my crds_cache. Could we alter the scrubbing behavior to "target scrub" the USER and HOSTNAME matching, e.g. by running a replace on the USER string with something like {\SCRUBBED USER_ID} ? |
Thanks for giving this a try. Are you asking about switching the "scrubbing" so that instead of removing the whole log line just replacing just the user name (for example) portion of the log line? So for a log line like the following:
The current PR would replace the entire log line with an empty string. Are you asking if instead we could replace it with the following?:
I didn't go with that approach because of possible inadvertent leaking of the user or hostname due to the string replace. Lets say ops decides to name the user "tim". The following line:
Would become:
which would leak the user name (on a line that otherwise would not contain that information). |
This is going to be tough to solve - one reason why I was hoping to avoid scrubbing in general... The reference file history is one of the most useful components of the log - it would be a shame to implement this with such a useful aspect missing. I'm not sure if it's worth attempting to implement a more "targeted vaccine", i.e. through string matching the user ID as part of a directory string, or if that just leads us further into the labyrinth. |
Can you give examples of log messages that are useful that were scrubbed?
Please do let me know if there are other approaches you'd like to explore. |
Here's a selection:
|
Thanks. The "Prefetch for" ones appear to be from: Line 97 in 382532d
(and possibly other log messages). The other log messages contain similar information and appear to be coming from steps like this one for the DARK reference:
Are the paths useful in either of these cases for a run on ops? For a local run the log messages that end up on the commandline shouldn't be touched by this PR. |
The log messages in the commandline are not affected, but I think having the stored logs out of sync with the command line logs is not an ideal situation. We do record the reference file names in the headers, but having the complete location is helpful in some cases. Perhaps it's not worth worrying about. |
Yeah, any scrubbing will result in the 2 being out of sync. If we need to scrub file paths we can't have the full paths for the reference files. At the moment the files contain no logs so having something is better than nothing I guess. I'm not prepared to argue that we shouldn't scrub the logs and will leave that up to you. |
After mulling it over, I think the keyword storage of reference file names is sufficient - we can re-visit the scrubbing if we receive negative feedback on missing entries. |
Builds off #8857
Resolves JP-3773
Closes #8855
From #8857
This PR extends that by:
Regression tests: https://github.com/spacetelescope/RegressionTests/actions/runs/13464767772
Tasks
Build 11.3
(use the latest build if not sure)no-changelog-entry-needed
)changes/
:echo "changed something" > changes/<PR#>.<changetype>.rst
(see below for change types)docs/
pageokify_regtests
to update the truth filesnews fragment change types...
changes/<PR#>.general.rst
: infrastructure or miscellaneous changechanges/<PR#>.docs.rst
changes/<PR#>.stpipe.rst
changes/<PR#>.datamodels.rst
changes/<PR#>.scripts.rst
changes/<PR#>.set_telescope_pointing.rst
changes/<PR#>.pipeline.rst
stage 1
changes/<PR#>.group_scale.rst
changes/<PR#>.dq_init.rst
changes/<PR#>.emicorr.rst
changes/<PR#>.saturation.rst
changes/<PR#>.ipc.rst
changes/<PR#>.firstframe.rst
changes/<PR#>.lastframe.rst
changes/<PR#>.reset.rst
changes/<PR#>.superbias.rst
changes/<PR#>.refpix.rst
changes/<PR#>.linearity.rst
changes/<PR#>.rscd.rst
changes/<PR#>.persistence.rst
changes/<PR#>.dark_current.rst
changes/<PR#>.charge_migration.rst
changes/<PR#>.jump.rst
changes/<PR#>.clean_flicker_noise.rst
changes/<PR#>.ramp_fitting.rst
changes/<PR#>.gain_scale.rst
stage 2
changes/<PR#>.assign_wcs.rst
changes/<PR#>.badpix_selfcal.rst
changes/<PR#>.msaflagopen.rst
changes/<PR#>.nsclean.rst
changes/<PR#>.imprint.rst
changes/<PR#>.background.rst
changes/<PR#>.extract_2d.rst
changes/<PR#>.master_background.rst
changes/<PR#>.wavecorr.rst
changes/<PR#>.srctype.rst
changes/<PR#>.straylight.rst
changes/<PR#>.wfss_contam.rst
changes/<PR#>.flatfield.rst
changes/<PR#>.fringe.rst
changes/<PR#>.pathloss.rst
changes/<PR#>.barshadow.rst
changes/<PR#>.photom.rst
changes/<PR#>.pixel_replace.rst
changes/<PR#>.resample_spec.rst
changes/<PR#>.residual_fringe.rst
changes/<PR#>.cube_build.rst
changes/<PR#>.extract_1d.rst
changes/<PR#>.resample.rst
stage 3
changes/<PR#>.assign_mtwcs.rst
changes/<PR#>.mrs_imatch.rst
changes/<PR#>.tweakreg.rst
changes/<PR#>.skymatch.rst
changes/<PR#>.exp_to_source.rst
changes/<PR#>.outlier_d 8000 etection.rst
changes/<PR#>.tso_photometry.rst
changes/<PR#>.stack_refs.rst
changes/<PR#>.align_refs.rst
changes/<PR#>.klip.rst
changes/<PR#>.spectral_leak.rst
changes/<PR#>.source_catalog.rst
changes/<PR#>.combine_1d.rst
changes/<PR#>.ami.rst
other
changes/<PR#>.wfs_combine.rst
changes/<PR#>.white_light.rst
changes/<PR#>.cube_skymatch.rst
changes/<PR#>.engdb_tools.rst
changes/<PR#>.guider_cds.rst