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-41561: Make bps restart accept various types of run IDs #25

Merged
merged 3 commits into from Nov 15, 2023

Conversation

mxk62
Copy link
Collaborator

@mxk62 mxk62 commented Nov 9, 2023

Checklist

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

Copy link

codecov bot commented Nov 9, 2023

Codecov Report

Attention: 32 lines in your changes are missing coverage. Please review.

Comparison is base (a0efba6) 15.48% compared to head (1cf24df) 15.24%.

Files Patch % Lines
python/lsst/ctrl/bps/htcondor/htcondor_service.py 3.03% 32 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #25      +/-   ##
==========================================
- Coverage   15.48%   15.24%   -0.25%     
==========================================
  Files           4        4              
  Lines        1330     1358      +28     
  Branches      281      290       +9     
==========================================
+ Hits          206      207       +1     
- Misses       1122     1149      +27     
  Partials        2        2              

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

Copy link
Contributor

@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.

Merge approved. Some minor comments.

The forbidden document start "---" could be fixed by removing those initial hyphens in that file.

doc/lsst.ctrl.bps.htcondor/userguide.rst Outdated Show resolved Hide resolved
doc/lsst.ctrl.bps.htcondor/userguide.rst Outdated Show resolved Hide resolved
doc/lsst.ctrl.bps.htcondor/userguide.rst Outdated Show resolved Hide resolved
python/lsst/ctrl/bps/htcondor/htcondor_service.py Outdated Show resolved Hide resolved
-------
wms_dir : `str`
Submit directory for the run with the given job id. If the submit
directory cannot be determined, it will be set to None.
Copy link
Contributor

Choose a reason for hiding this comment

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

What does "cannot be determined" mean? Does the method guarantee that a directory exists and is readable?

Copy link
Collaborator Author

@mxk62 mxk62 Nov 14, 2023

Choose a reason for hiding this comment

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

The submit directory can't be determined if the user provided a local job id as the run id, but there are no entries for this run in the HTCondor history file either because it rolled over or we're querying a wrong scheduler because, for example, the user ran bps restart on sdfrome02 while the workflow was initially submitted to execution on sdfrome01. Though I just realized that I removed the check if directory exists and is readable from the caller without putting it here. Will fix.

doc/lsst.ctrl.bps.htcondor/userguide.rst Outdated Show resolved Hide resolved
@mxk62 mxk62 merged commit 13fd706 into main Nov 15, 2023
9 of 11 checks passed
@mxk62 mxk62 deleted the tickets/DM-41561 branch November 15, 2023 18:57
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.

None yet

2 participants