celery / jumpstarter

MIT License
7 stars 3 forks source link

Actor Tasks #24

Open thedrow opened 3 years ago

thedrow commented 3 years ago

Implementation of actor tasks.

Fixes #7.

ghost commented 3 years ago

DeepCode's analysis on #d91485 found:

Description Example fixes
Access to a protected member _state_machine of a client class Occurrences: :wrench: Example fixes
Unused partial imported from functools Occurrences: :wrench: Example fixes

šŸ‘‰ View analysis in DeepCodeā€™s Dashboard | Configure the bot

lgtm-com[bot] commented 3 years ago

This pull request introduces 1 alert when merging 89bd0a4c3a2ae939ae04be6bc0da5fe01c1e7176 into c13b4baf3fb3137888cbb4cd66fe989d0899feed - view on LGTM.com

new alerts:

ahopkins commented 3 years ago

@thedrow It looks like there are two potential solutions to making ActorStartedState.healthy parallel to foo_taskā†¦running

  1. all of foo_task is made a substate of started instead of being a top-level state of the root level state machine.
  2. ActorStartedState.healthy is pulled into foo_task as a substate of running so that running is itself a parallel state with ActorStartedState.healthy and foo_taskā†¦running.

The second option would look something like this:

INFO:transitions.extensions.asyncio:Finished processing state started exit callbacks.
INFO:transitions.extensions.asyncio:Finished processing state foo_task enter callbacks.
INFO:transitions.extensions.asyncio:Finished processing state foo_taskā†¦initialized enter callbacks.
INFO:transitions.extensions.asyncio:Finished processing state foo_taskā†¦initialized exit callbacks.
INFO:transitions.extensions.asyncio:Finished processing state foo_taskā†¦running enter callbacks.
INFO:transitions.extensions.asyncio:Finished processing state foo_taskā†¦runningā†¦healthy enter callbacks.
INFO:transitions.extensions.asyncio:Finished processing state foo_taskā†¦runningā†¦running enter callbacks.
> my_actor.state=[<ActorStartedState.healthy: 1>, <TaskState.running: 2>]

Option two is easier to achieve with the current PR (albeit, probably need some cleanup on the double naming of running, but this at least shows the idea):

    def __set_name__(self, owner, name):
        ...
        healthy = state_machine.get_state(ActorStartedState.healthy)
        self.running_state.add_substate(healthy)
        self.running_state.add_substate(NestedAsyncState(TaskState.running))
        self.running_state.initial = [ActorStartedState.healthy, TaskState.running]

Option one requires a little more work to rearrange the task_state. I guess the question is whether healthy must be a substate of started, or if we can simply have assert ActorStartedState.healthy in my_actor.state.

ahopkins commented 3 years ago

Separate question, is there a specific reason for using exceptions instead of events?

@task
async def foo(self):
    raise Success()

@task
async def bar(self, event):
    event.set()
lgtm-com[bot] commented 3 years ago

This pull request fixes 1 alert when merging d914858cdd0b2eff3bf5f00671ecdc499eb8a8f9 into 583f6492ba0e67d4a19a763ee905193322afa8a4 - view on LGTM.com

fixed alerts:

thedrow commented 3 years ago

Separate question, is there a specific reason for using exceptions instead of events?

@task
async def foo(self):
    raise Success()

@task
async def bar(self, event):
    event.set()

It's an API I came up with. We can change it. It's exceptional in my opinion since we finish the task.