Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix typing for Actor's join() method #45

Closed
leandro-lucarella-frequenz opened this issue Oct 19, 2022 · 1 comment · Fixed by #564
Closed

Fix typing for Actor's join() method #45

leandro-lucarella-frequenz opened this issue Oct 19, 2022 · 1 comment · Fixed by #564
Assignees
Labels
part:actor Affects an actor ot the actors utilities (decorator, etc.) priority:low This should be addressed only if there is nothing else on the table type:bug Something isn't working
Milestone

Comments

@leandro-lucarella-frequenz
Copy link
Contributor

leandro-lucarella-frequenz commented Oct 19, 2022

What happened?

When calling the join() method of an actor (or _stop() or probably any method defined by the @actor decorator) we get an error from mypy complaining about the method not existing in the actor class, for example:

test.py:10: error: "SampleActor" has no attribute "join"

pylint seems to be complaining too.

What did you expect instead?

No errors when calling the method.

Affected version(s)

All

Affected part(s)

Actors

Extra information

The typing_extensions package (#19), in particular the @dataclass_transform() decorator, might help with this, it needs further investigation.

As a workaround, # noqa can be used when the method is called.

Here is an example where we need to use # noqa:

await distributor._stop() # type: ignore # pylint: disable=no-member

@leandro-lucarella-frequenz leandro-lucarella-frequenz added priority:low This should be addressed only if there is nothing else on the table type:bug Something isn't working part:actor Affects an actor ot the actors utilities (decorator, etc.) labels Oct 19, 2022
@keywordlabeler keywordlabeler bot added the part:❓ We need to figure out which part is affected label Oct 19, 2022
@leandro-lucarella-frequenz leandro-lucarella-frequenz removed the part:❓ We need to figure out which part is affected label Oct 19, 2022
@leandro-lucarella-frequenz
Copy link
Contributor Author

Comment by @jakub-toptal

Perhaps we can experiment in pulling that method to the BaseActor class to circumvent it. Otherwise, we could also implement:

async def join() -> None:
     await super().join()

but it would have to be repeated for every actor class so I guess ignoring the error is already better.

EDIT: I tried to pull that method to the BaseActor class and to somehow specify the type returned by the@actor decorator to inherit from BaseActor, but it didn't work and the pylint warning is still there.

@llucax llucax modified the milestones: v0.24.0, v0.25.0 Aug 1, 2023
@llucax llucax moved this from To do to Review in progress in Python SDK Roadmap Aug 15, 2023
github-merge-queue bot pushed a commit that referenced this issue Aug 25, 2023
`BackgroundService` is a new abstract base class can be used to write
other classes that runs one or more tasks in the background. It provides
a consistent API to start and stop these services and also takes care of
the handling of the background tasks. It can also work as an `async`
context manager, giving the service a deterministic lifetime and
guaranteed cleanup.

The new `Actor` class brings quite a few new improvements over the old
`@actor` decorator. These are the main differences:

* It doesn't start automatically, `start()` needs to be called to start
an actor.
* The method to implement the main logic was renamed from `run()` to
`_run()`, as it is not intended to be run externally.
* Actors can have an optional `name` (useful for debugging/logging
purposes) and `loop` (if the actor should run in a loop different from
the currently running loop).
* The actor will only be restarted if an unhandled `Exception` is raised
by `_run()`. It will not be restarted if the `_run()` method finishes
normally. If an unhandled `BaseException` is raised instead, it will be
re-raised. For normal cancellation the `_run()` method should handle
`asyncio.CancelledError` if the cancellation shouldn't be propagated
(this is the same as with the decorator).
* The `_stop()` method is public (`stop()`) and will `cancel()` and
`await` for the task to finish, catching the `asyncio.CancelledError`.
* The `join()` method is renamed to `wait()`, but they can also be
awaited directly ( `await actor`).
* For deterministic cleanup, actors can now be used as `async` context
managers.

The base actors (`ConfigManagingActor`,
`ComponentMetricsResamplingActor`, `DataSourcingActor`,
`PowerDistributingActor`) now inherit from the new `Actor` class, as
well as the `MovingWindow`.

Fixes #240, fixes #45, fixes #196.
@github-project-automation github-project-automation bot moved this from Review in progress to Done in Python SDK Roadmap Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
part:actor Affects an actor ot the actors utilities (decorator, etc.) priority:low This should be addressed only if there is nothing else on the table type:bug Something isn't working
Projects
Development

Successfully merging a pull request may close this issue.

3 participants