[pycue/cuegui/cuebot] Add Department wrapper and fix TasksDialog (#2399)#2427
[pycue/cuegui/cuebot] Add Department wrapper and fix TasksDialog (#2399)#2427ttpss930141011 wants to merge 5 commits into
Conversation
The DepartmentInterface RPC was declared as SetMangedCores while the cuebot servant implements setManagedCores. The servant method never overrode the generated base method, so the RPC always returned UNIMPLEMENTED. Rename the RPC to the correct spelling and mark the servant method as an override. The rename is safe: no client could ever call this RPC successfully, and nothing else references the old name. Part of AcademySoftwareFoundation#2399.
CueGUI's TasksDialog calls Show.getDepartments() and several methods on the returned department objects, but pycue never had a Department wrapper, so the dialog raised AttributeError on open (AcademySoftwareFoundation#1042). - Add opencue.wrappers.department.Department covering the DepartmentInterface RPCs that cuebot implements. - Add Show.getDepartment() and Show.getDepartments(). - Add api.addDepartmentName() and api.removeDepartmentName() alongside the existing getDepartmentNames() so the list of allowed department names can be managed from Python. - Add Task.clearAdjustments(), used by the Tasks dialog context menu. DepartmentInterface.Delete is intentionally not wrapped because cuebot does not implement it. Fixes AcademySoftwareFoundation#2399
TaskActions.clearAdjustment called task.clearAdjustment(), which never existed on the pycue Task wrapper; the unit test mocked the attribute so the gap was never caught. Point both at the real Task.clearAdjustments() method. Part of AcademySoftwareFoundation#2399.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughAdds a new ChangesDepartment API wrappers and related fixes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thanks for your contribution, @ttpss930141011 We will review your PR soon. |
DiegoTavares
left a comment
There was a problem hiding this comment.
Thanks for the contribution
|
This change should be merged next week after I find some time to test it out in our environment. |
Related Issues
Resolves #2399
Originally reported as #1042
Related (not addressed here): #2400
Summarize your change.
CueGUI's
TasksDialogis written entirely against the pycue wrapper API, but theDepartmentwrapper it depends on was never implemented — so opening the dialog raisedAttributeErroronShow.getDepartments()(TasksDialog.py:94). This adds the missing wrapper layer and fixes two related gaps found while wiring it up.pycue
opencue/wrappers/department.py:Departmentwrapper following the existingtask.pypattern (self.data+self.stub, one method per RPC), covering theDepartmentInterfaceRPCs that cuebot implements:addTask,addTasks,clearTasks,clearTaskAdjustments,enableTiManaged,disableTiManaged,getTasks,replaceTasks,setManagedCores.DepartmentInterface.Deleteis intentionally not wrapped — cuebot has no implementation for it.Show.getDepartment()/Show.getDepartments(), mirroring the existinggetGroups().api.addDepartmentName()/api.removeDepartmentName(), alongside the existinggetDepartmentNames().Task.clearAdjustments(), used by the Tasks dialog context menu.cuegui
TaskActions.clearAdjustmentcalledtask.clearAdjustment(), which never existed on the wrapper; the unit test mocked the attribute, so the gap was never caught. Both now call the realTask.clearAdjustments().cuebot / proto
rpc SetMangedCores(typo) while the servant implementssetManagedCores. Because the names differed, the servant never overrode the generated base method and the RPC always returnedUNIMPLEMENTED— so the "Minimum Cores" control inTasksDialogwould fail even with the wrapper in place. Renamed the RPC toSetManagedCoresand added@Overrideon the servant. The rename is safe: no client could have called the old RPC successfully, and nothing references the old name.VERSION.inis bumped 1.25 → 1.26 as required byci/check_version_bump.pyfor proto changes.Tests: new
tests/wrappers/test_department.py, plusgetDepartment/getDepartmentsintest_show.py,clearAdjustmentsintest_task.py, andgetDepartmentNames/addDepartmentName/removeDepartmentNameintest_api.py.Open question for the reviewer: #2400 suggested naming these
createDepartment/deleteDepartment. I choseaddDepartmentName/removeDepartmentNameinstead, to match the underlying RPC names and the existinggetDepartmentNames(), and because they manage the allowed-name whitelist rather than creating a show's department entity (entities are auto-created when a name is assigned to a group — seedepartment.proto). Happy to rename if thecreate/deletespelling is preferred.LLM usage disclosure
Claude (Opus) was used to build up testing environment across cuegui/pycue/cuebot, write the unit tests, and draft this PR description.
Summary by CodeRabbit
Release Notes v1.26
New Features
Bug Fixes
Documentation