Changes to job manager queue limit and queued_for_start#873
Changes to job manager queue limit and queued_for_start#873VictorVerhaert wants to merge 3 commits intomasterfrom
Conversation
|
can you link to the tickets/PRs that provide more context about these changes? |
|
pr: #872 |
|
can you link those in the changelog? |
|
or is that a no go? |
|
There was no issue created for this behavior so nothing to link, but rather a bug found last week and this fix solved it for me. |
|
I'm a bit confused by the introduced change to be honest #872 (review) |
VictorVerhaert
changed the title
VictorVerhaert
force-pushed
the
small-jm-fixes
branch
from
8517a04 to
5284e25
Compare
VictorVerhaert
force-pushed
the
small-jm-fixes
branch
from
6976b42 to
61317c1
Compare
| f"Status of job {job_id!r} (on backend {backend_name}) is {new_status!r} (previously {previous_status!r})" | ||
| ) | ||
|
|
||
| if new_status == "created" and previous_status == "queued_for_start": |
There was a problem hiding this comment.
I would even put this immediately after new_status = job_metadata["status"], because now the logging will be inconsistent with the real value of the new_status value
or you could add a _log.warning that you are overruling the status here
| @@ -548,7 +548,7 @@ def _job_update_loop( | |||
| # TODO: should "created" be included in here? Calling this "running" is quite misleading then. | |||
| # apparently (see #839/#840) this seemingly simple change makes a lot of MultiBackendJobManager tests flaky | |||
| running = job_db.get_by_status(statuses=["created", "queued", "queued_for_start", "running"]) | |||
There was a problem hiding this comment.
from discussion with @VictorVerhaert; perhaps we can remove created here now as we later on fix the state
3 participants
pr this refers to: #872