Remove JobQueueManager #6452
Remove JobQueueManager #6452
Conversation
berland
added
the
release-notes:maintenance
berland
force-pushed
the
remove_jobqueuemanager
branch
2 times, most recently
from
e2034d9
to
b191198
Compare
Codecov Report
@@ Coverage Diff @@
## main #6452 +/- ##
==========================================
- Coverage 83.41% 83.36% -0.05%
==========================================
Files 343 342 -1
Lines 20806 20769 -37
Branches 944 944
==========================================
- Hits 17356 17315 -41
- Misses 3163 3166 +3
- Partials 287 288 +1
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
berland
force-pushed
the
remove_jobqueuemanager
branch
from
b191198
to
19d4067
Compare
| @@ -49,6 +49,11 @@ | |||
| logger = logging.getLogger(__name__) | |||
|
|
|||
| LONG_RUNNING_FACTOR = 1.25 | |||
| """If STOP_LONG_RUNNING is true, realizations taking more time than the average | |||
There was a problem hiding this comment.
Just realized that these attribute docstrings are not standardized, and cannot be picked up by any tool.
| while self.is_active() and not self.stopped: | ||
| self.launch_jobs(pool_sema) | ||
| self.launch_jobs(self._pool_sema) |
There was a problem hiding this comment.
This parameter being an instance variable; can it be moved into the launch_jobs method directly instead of passed in as a parameter?
There was a problem hiding this comment.
Yes it can. Well spotted.
There was a problem hiding this comment.
Nope, we need it to retain the possibility to set it when running from _builder/_legacy.py
There was a problem hiding this comment.
Ahh, because the execute_queue_via_websockets use the one from legacy. Got it!
|
|
||
| def getNumPending(self) -> int: | ||
| return self._queue_manager.getNumPending() | ||
| return self._job_queue.count_status(JobStatus.PENDING) # type: ignore |
There was a problem hiding this comment.
What is the reason for all the type: ignore? Will this be fixed in a separate PR/issue?
There was a problem hiding this comment.
These are due to difficulties with integrating the typing with the C-code and the custom cwrap (not Pybind). It will go away as soon as we have thrown away the C-code..
| while self.is_active() and not self.stopped: | ||
| self.launch_jobs(pool_sema) | ||
| self.launch_jobs(self._pool_sema) |
There was a problem hiding this comment.
Ahh, because the execute_queue_via_websockets use the one from legacy. Got it!
It was only a thin wrapper with no value left. Move its functionality down to the JobQueue object.
berland
force-pushed
the
remove_jobqueuemanager
branch
from
4875731
to
07b9e87
Compare
It was only a thin wrapper with no value left. Move its functionality down to the JobQueue object.
Issue
Resolves #6285
Approach
Short description of the approach
(Screenshot of new behavior in GUI if applicable)
Pre review checklist
Ground Rules),
and changes to existing code have good test coverage.
Pre merge checklist