Move transitions to enum #35
Conversation
Codecov Report
@@ Coverage Diff @@
## actor #35 +/- ##
==========================================
+ Coverage 94.54% 94.79% +0.25%
==========================================
Files 5 5
Lines 458 480 +22
Branches 62 62
==========================================
+ Hits 433 455 +22
Misses 18 18
Partials 7 7
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
👉 View analysis in DeepCode’s Dashboard | Configure the bot |
| @@ -409,7 +454,7 @@ async def _release_resources(event_data: transitions.EventData) -> None: | |||
| async def _maybe_set_event(event_data: EventData, event_name: str) -> None: | |||
| kwargs = _merge_event_data_kwargs(event_data) | |||
| try: | |||
| event: Event = kwargs[event_name] | |||
| event: EventType = kwargs[event_name] | |||
There was a problem hiding this comment.
That's definitely not correct. This is an AnyIO event.
There was a problem hiding this comment.
I renamed the anyio.Event at import to not conflict with the Event enum. It seemed to me that the having a variable Event.bootup and Event.shutdown was more useful than the type annotation. By changing the name of the annotation at import, mypy will still work, and IDEs will still prompt you to use the correct value. It just avoids the name collision.
There was a problem hiding this comment.
from anyio.abc import Event as EventType
There was a problem hiding this comment.
I think we should rename that enum to ActorLifecycleEvent.
That would be more helpful.
| @@ -34,6 +34,34 @@ | |||
| NestedState.separator = "↦" | |||
|
|
|||
|
|
|||
| class Transition(str, Enum): | |||
There was a problem hiding this comment.
These are triggers, not transitions.
| restarting__starting = auto() | ||
| restarting__stopping = auto() |
| stop = auto() | ||
|
|
||
|
|
||
| class Event(str, Enum): |
There was a problem hiding this comment.
I think we're using the wrong terminology here.
It could be because my design isn't 100% correct as of yet.
There was a problem hiding this comment.
I think we should rename that enum to
ActorLifecycleEvent.
That would be more helpful.
Putting all of the transition strings into an enum should make it easier to manage going forward.