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-46307: execute butler housekeeping scripts at remote DF for multisite processing #186

Merged
merged 17 commits into from
Dec 10, 2024

Conversation

mxk62
Copy link
Contributor

@mxk62 mxk62 commented Nov 21, 2024

Checklist

  • ran Jenkins
  • added a release note for user-visible changes to doc/changes

Copy link

codecov bot commented Nov 21, 2024

Codecov Report

Attention: Patch coverage is 68.46154% with 82 lines in your changes missing coverage. Please review.

Project coverage is 82.57%. Comparing base (a05d1e7) to head (4d9f944).
Report is 18 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
python/lsst/ctrl/bps/drivers.py 18.86% 43 Missing ⚠️
python/lsst/ctrl/bps/initialize.py 41.37% 31 Missing and 3 partials ⚠️
python/lsst/ctrl/bps/construct.py 90.24% 2 Missing and 2 partials ⚠️
python/lsst/ctrl/bps/cli/cmd/commands.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #186      +/-   ##
==========================================
- Coverage   82.58%   82.57%   -0.02%     
==========================================
  Files          41       44       +3     
  Lines        3492     3672     +180     
  Branches      355      359       +4     
==========================================
+ Hits         2884     3032     +148     
- Misses        518      549      +31     
- Partials       90       91       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mxk62 added 10 commits November 23, 2024 12:58
The code responsible for creating the submit directory was rather
generic. I factored it out from the _init_submission_driver() and added
it as a general purpose function to bps_utils.
Moved the "business logic" out of the _init_submission_driver()
to a separate function to make the driver its structure more similar to
other drivers.
Validating a BPS configuration for a regular workflow and for a
workflow executing a script remotely requires different sets of
validation tests.  However, the validation tests for BPS config
performed in init_submission() were "hard-coded" and there was no
mechanism allowing for customizing them.  Modified this function so
the validation tests it performs could be explicitly specified when
called.
Added a new command that will allow the user to submit custom
scripts for execution.
Some unit tests for _init_submission_driver() stopped working after
refactoring.  Made some modifications to make them work again.
Added a new workflow attribute, bps_iscustom, so any BPS plugin can
easily distinguish between regular payload workflows and custom ones
intended to run ad hoc scripts.
Added a validator that will ensure 'customJob" is defined in
BPS config when submitting a custom jobs.
@mxk62 mxk62 force-pushed the tickets/DM-46307 branch 2 times, most recently from ced56c0 to 03b03c7 Compare November 23, 2024 20:12
Copy link
Collaborator

@MichelleGower MichelleGower left a comment

Choose a reason for hiding this comment

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

Some suggestions. Merge approved

doc/lsst.ctrl.bps/quickstart.rst Outdated Show resolved Hide resolved
doc/lsst.ctrl.bps/quickstart.rst Outdated Show resolved Hide resolved
doc/lsst.ctrl.bps/quickstart.rst Outdated Show resolved Hide resolved
doc/lsst.ctrl.bps/quickstart.rst Outdated Show resolved Hide resolved
doc/lsst.ctrl.bps/quickstart.rst Show resolved Hide resolved
doc/lsst.ctrl.bps/quickstart.rst Outdated Show resolved Hide resolved
doc/lsst.ctrl.bps/quickstart.rst Show resolved Hide resolved
python/lsst/ctrl/bps/initialize.py Show resolved Hide resolved
python/lsst/ctrl/bps/drivers.py Show resolved Hide resolved
python/lsst/ctrl/bps/drivers.py Outdated Show resolved Hide resolved
mxk62 added 6 commits December 4, 2024 16:17
Based on the reviewer input, I fixed some spelling errors in the documentation
and included additional pieces of information that the user might be interested
in.
Some new docstrings had typos and some shortcomings (e.g. missing return
value description).  Other were missing entirely.  Fixed all of those.
While the custom job validator was checking if the required section in
BPS config exists, it wasn't checking if the required entry, in this
section, i.e.,  ``executable``, is present.  Made sure it does it as
well.
Made some minor edits to the log messages displayed during BPS
submission to make them either more specific or stylistically
consistent.
The same snippet responsible for logging memory usage was repeated
multiple times in ``drivers.py`` module.  Factord it out as an auxiliary
function to avoid repetitions.
Added an option the will allow the user to specify compute site
when calling ``bps submitcmd``.
@mxk62 mxk62 merged commit c55a5d2 into main Dec 10, 2024
13 of 15 checks passed
@mxk62 mxk62 deleted the tickets/DM-46307 branch December 10, 2024 19:20
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