feat: Add event_listeners module for event lifecycle support#203
feat: Add event_listeners module for event lifecycle support#203morgan-wowk wants to merge 1 commit intoenums-refactorfrom
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
morgan-wowk
force-pushed
the
event-system
branch
2 times, most recently
from
f851682 to
38ecf70
Compare
morgan-wowk
changed the title
Provides a general-purpose publish/subscribe system with: - Event marker dataclass base (kw_only=True) enforced via TypeVar bound - subscribe(event_type, callback, asynchronous=True) keyed by event class; async subscribers run on a daemon thread, sync on the calling thread - emit(event) dispatches to all subscribers by type(event) Co-located tests cover sync/async dispatch, type isolation, and that asynchronous=True is the default.
morgan-wowk
force-pushed
the
graphite-base/203
branch
from
364adae to
fd0568a
Compare
morgan-wowk
force-pushed
the
event-system
branch
from
38ecf70 to
09e430b
Compare
|
|
||
| _EventType = typing.TypeVar("_EventType", bound=Event) | ||
|
|
||
| _listeners: dict[type, list[_CallbackEntry]] = {} |
There was a problem hiding this comment.
FYI - there is typing.Final which will have linting to make sure this variable can't be assigned. Not necessary needed here, but just an FYI when I create global variables.
| @@ -0,0 +1,45 @@ | |||
| import dataclasses | |||
| import threading | |||
| import typing | |||
There was a problem hiding this comment.
Google style guide for typing would be to not import it as a module, I believe we do that in other places in this repo as well.
https://google.github.io/styleguide/pyguide.html#31912-imports-for-typing
| if asynchronous: | ||
| threading.Thread(target=callback, args=(event,), daemon=True).start() | ||
| else: | ||
| callback(event) |
There was a problem hiding this comment.
I believe wrapping a try/catch here would be important to not break the emit for loop?
Add a new test for this too.
There was a problem hiding this comment.
Hmm...I guess that would help ensure at least the "async" listeners are guarenteed. But a synchronous listener should prevent any further action / subsequent synchronous listners ffrom executing.
Does that sound good? We add a try catch to ensure the async listeners are always called?
There was a problem hiding this comment.
We could also just dispatch the asychronous listeners first instead of having a try catch
2 participants
Provides a general-purpose publish/subscribe system with:
ContainerExecutionStatus fields