Add EventSender to resolve dependency cycle
#6864
Conversation
pinkwah
force-pushed
the
cyclic
branch
2 times, most recently
from
23f0fdb to
78b12d3
Compare
|
This needs rebasing first |
|
Can this solve Lines 28 to 29 in 881a350 |
pinkwah
force-pushed
the
cyclic
branch
3 times, most recently
from
e3682ca to
a80bea8
Compare
| self._jobs: MutableMapping[int, Job] = { | ||
| real.iens: Job(self, real) for real in (realizations or []) | ||
| self._jobs: Mapping[int, Job] = { | ||
| real.iens: Job(real, on_complete=self._on_job_complete) |
There was a problem hiding this comment.
Looks good! But (just our of curiosity mostly), why not to to use:
self._tasks[iens].add_done_callback(self._on_job_complete)?
There was a problem hiding this comment.
Because it's not when the task is done running, it's when the job is COMPLETED successfully.
There was a problem hiding this comment.
Yes, but we can check in _on_job_complete that self.state == COMPLETED?
pinkwah
force-pushed
the
cyclic
branch
5 times, most recently
from
a955563 to
f7f4a56
Compare
src/ert/scheduler/event_sender.py
Outdated
| ): | ||
| while True: | ||
| event = await self.events.get() | ||
| print(f"==SENDING {event=}") |
There was a problem hiding this comment.
leftover print statement?
| async def _submit_and_run_once(self, sem: asyncio.BoundedSemaphore) -> None: | ||
| async def _submit_and_run_once( | ||
| self, sem: asyncio.BoundedSemaphore, driver: Driver | ||
| ) -> None: |
There was a problem hiding this comment.
is this driver arg related to the commit message, or is it some other refactoring?
There was a problem hiding this comment.
It's part of the commit. The Job object obtains a Driver only when it is supposed to use it, not before.
pinkwah
force-pushed
the
cyclic
branch
4 times, most recently
from
42e8ebb to
f34acb9
Compare
`Scheduler` has a reference to `Job` and `Job` has a reference to `Scheduler`. Adding `EventSender` lets us resolve this cycle as now `Scheduler` has a reference to `Job`s and `EventSender`, but each `Job` only refers to `EventSender`.
|
Closing as this doesn't really solve anything. There will be a cyclic dependency regardless. |
Schedulerhas a reference toJobandJobhas a reference toScheduler. AddingEventSenderlets us resolve this cycle as nowSchedulerhas a reference toJobs andEventSender, but eachJobonly refers toEventSender.