DM-42530: changes for automatic Large Glideins #25
Conversation
MichelleGower
changed the title
| ] | ||
| owner = f'(Owner=="{auser}") ' | ||
| jstat = "&& (JobStatus==1) " | ||
| bps1 = "&& (bps_run isnt Undefined) " |
There was a problem hiding this comment.
Took me a while to find "isnt" in the documentation. In case someone else needs the link: https://htcondor.readthedocs.io/en/latest/classads/classad-mechanism.html#expression-operators
| schedd_name = socket.getfqdn() | ||
| coll = htcondor.Collector() | ||
| schedd_ad = coll.locate(htcondor.DaemonTypes.Schedd) | ||
| scheddref = htcondor.Schedd(schedd_ad) |
There was a problem hiding this comment.
To my understanding these "preparations" (lines 221 - 224) are made only to call condor_q() later in line 260. However, it looks like the code is concerned with identifying the local Schedd only. As a result, it is a bit redundant as condor_q() can do it for you if you omit the (optional) schedds argument.
|
|
||
| verbose = self.isVerbose() | ||
| if len(condorq_data) == 0: |
There was a problem hiding this comment.
Empty Python containers are false, so the preferred way to check if condor_data contains any elements would be
if not condor_data:
| print("Auto: No Large BPS Jobs.") | ||
| return | ||
|
|
||
| if len(condorq_data) > 0: |
There was a problem hiding this comment.
I don't think this check is necessary (or alternatively the previous one in line 266). If condor_data contains no elements we will never reach this point because of the return statement in line 268 and containers size can't be a negative number.
| print(f"\n{job_label}") | ||
| existingGlideinsIdle = 0 | ||
| numberOfGlideinsReduced = 0 | ||
| numberOfGlideins = 0 |
There was a problem hiding this comment.
I'm not sure why we need to set existingGlideinsIdle, numberOfGlideinsReduced, and numberOfGlideins to zero here. Each of these variables seems to be initialized with a different value later (see lines 320, 334, and 339, respectively).
| large = f"&& (RequestMemory>{memoryLimit} || RequestCpus>{autoCPUs})" | ||
| # The constraint determines that the jobs to be returned belong to | ||
| # the current user (Owner) and are Idle vanilla universe jobs. | ||
| full_constraint = f"{owner}{jstat}{bps1}{bps2}{juniv}{large}" |
There was a problem hiding this comment.
I think making owner, jstat, etc. just simple clauses that is
owner = f'(Owner=="{auser}")'
jstat = f"(JobStatus=={htcondor.JobStatus.IDLE})"
...
and writing the full constraint as
full_constraint = f"{owner} && {jstat} && {bps1} && {bps2} && {juniv} && {large}"
would make the code easier to read.
No description provided.