DM-37020: Replace "requirements" with "constraint" in HTCondor Python function calls in ctrl_bps_htcondor #19
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #19 +/- ##
==========================================
+ Coverage 15.38% 15.62% +0.24%
==========================================
Files 4 4
Lines 1313 1318 +5
Branches 279 276 -3
==========================================
+ Hits 202 206 +4
- Misses 1109 1110 +1
Partials 2 2
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
It probably would be better to support both versions. If none of the external LSST folks use the older HTCondor, then it won't matter. For a future ticket, we may want to add a version check at the beginning (older than X not supported, X-Y supported, and warning Y+ may or may not work)
|
Do we have to change the pin in |
|
The changes I made need HTCondor version 9.0 or greater to work. So yes, we should update the minimum required versions in |
mxk62
force-pushed
the
tickets/DM-37020
branch
from
1263a53
to
af5a823
Compare
| for job_ad in query.nextAdsNonBlocking(): | ||
| yield schedd_name, job_ad | ||
|
|
||
|
|
There was a problem hiding this comment.
Which version does the htc_version return if python API is different than service version? If this returns python API version, do we want to change this to use packaging.version? If this actually returns the service version, please change the docstring to be more explicit.
There was a problem hiding this comment.
As far as I can tell htcondor.version() (and in turn htc_version()) returns the client side version of HTCondor API in use. So if you don't object, I can rewritehtc_version() to just return HTC_VER instead of parsing the output of the htcondor.version() or get rid of it entirely (it is used once).
| def condor_q(constraint=None, schedds=None): | ||
| """Query HTCondor for current jobs. | ||
| def condor_q(constraint=None, schedds=None, **kwargs): | ||
| """Get information about running jobs from HTCondor. |
There was a problem hiding this comment.
It doesn't return information about pending jobs?
There was a problem hiding this comment.
It does. Wrong wording. Unless you have a better suggestion, I'll change it to
Get information about the jobs in HTCondor job queue(s).
| job_info[schedd_name][id_] = dict(job_ad) | ||
| _LOG.debug("condor_q returned %d jobs", sum(len(val) for val in job_info.values())) | ||
| def condor_history(constraint=None, schedds=None, **kwargs): | ||
| """Get information about completed jobs from HTCondor history. |
There was a problem hiding this comment.
Nitpicking about word again (that was the same way in code previously). "completed" It includes deleted jobs too?
There was a problem hiding this comment.
I'll change it to
Get information about the jobs from HTCondor history records.
The named argument in `htcondor.Schedd` methods, `requirements' was deprecated by HTCondor team in favor of `constraint` and was triggering deprecation warnings, e.g., when one was using `bps report`. Replaced it with its contemporary equivalent to get rid of those warnings.
By popular demand (see https://ls.st/090 for details) I removed the try-except guard in module's __init__.py.
htc_version() was parsing the string returned by htcondor.version(). Now it just returns a string representation of HTC_VERSION.
mxk62
force-pushed
the
tickets/DM-37020
branch
from
c14ac46
to
5dde25f
Compare
Checklist
doc/changes