[cuegui] Bug fix missing jobs on MonitorJobs #1312
Conversation
When the option group dependent is checked, jobs without dependency are being omitted.
ramonfigueiredo
requested review from
DiegoTavares,
IdrisMiles,
bcipriano,
gregdenton,
jrray,
larsbijl,
smith1511 and
splhack
as code owners
| ).getRecursiveDependentJobs([newJobObj], | ||
| active_only=active_only) | ||
|
|
||
| # Remove dependent if it has the same name as the job |
There was a problem hiding this comment.
Could you explain this fix a bit more? In the PR you say "jobs without dependency are being omitted" but it's not clear to me how this change solves the problem. Just trying to understand.
There was a problem hiding this comment.
Hi @bcipriano
The changes in this new PR is a fix for a bug created in the following PRs:
Dependent feature
#815
Add kill dependent and group dependent features
#1115
There was a feature implementation that added the group-dependent features on the CueGUI.
To reproduce the error go to:
CueGUI > Monitor Jobs and check the checkbox "[x] Group Dependent"
- Some jobs that had dependents were not shown in the Monitor Job.
However, if you uncheck the checkbox "[] Group Dependent" the jobs are displayed correctly.
After get the dependent jobs list that wrongly included the parent job name:
JobMonitorTree.py > class JobMonitorTree(cuegui.AbstractTreeWidget.AbstractTreeWidget): > def addJob(self, job, timestamp=None, loading_from_config=False):
dep = self.__menuActions.jobs()
.getRecursiveDependentJobs([newJobObj], active_only=active_only)
The bug was happening because the this for:
JobMonitorTree.py > class JobMonitorTree() > def addJob():
for j in self.__reverseDependents:
if j in self.__load:
del self.__load[j]
The __reverseDependents had the name of the parent job and when the del self.__load[j] is executed, the CueGui was removing the parent job and all the dependent jobs, resulting on missing jobs on MonitorJobs.
There was a problem hiding this comment.
Hi @bcipriano
I sent a new version using list comprehension to avoid many nested blocks and follow the project code style.
Lint Python Code error:
https://github.com/AcademySoftwareFoundation/OpenCue/actions/runs/5942566910/job/16118316707
Running lint for cuegui/...
************* Module cuegui.JobMonitorTree
cuegui/JobMonitorTree.py:262:8: R1702: Too many nested blocks (6/5) (too-many-nested-blocks)
- Remove dependent if it has the same name as the job. This avoids missing jobs on MonitorJobs. Remove the parent job is necessary to avoid remove the parent job and all the dependents when del self.__load[j] is called - Use list comprehension to remove dependent if it has the same name as the job - List comprehension avoid many nested blocks and follow the project code style
- AbstractTreeWidget.py > _removeItem(): Check if object ID exists before delete it
bcipriano
left a comment
bcipriano
left a comment
There was a problem hiding this comment.
Thanks for the explanation. LGTM
bcipriano
left a comment
bcipriano
left a comment
There was a problem hiding this comment.
@ramonfigueiredo Let's merge master on this PR too so the tests can run again. Looks like they expired. Sorry for the delay.
- When the option group dependent is checked, jobs without dependency are omitted. - Code refactoring
|
Done! Ready to merge! |
3 participants
When the option group dependent is checked, jobs without dependency are being omitted.