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

Akka-style message Scheduler #95

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

Conversation

akatashev
Copy link

@akatashev akatashev commented May 16, 2020

First of all, thanks for Pykka, it's really nice!
There is this issue about auto-scheduling: #53

I needed this functionality for my project as well, because I believe, that an ability to use schedulers to retransmit messages, schedule job executions and so on, is quite a bit part of Akka, so I created it for myself and I hope, it could be useful for Pykka community as well.

Basically, I implemented the simplest versions of Cancellable, Scheduler.scheduleOnce, Scheduler.scheduleWithFixedDelay and Scheduler.scheduleAtFixedRate from Akka.

There are 3 schedulers for 3 different kinds of Actors:
ThreadingScheduler - based on threading.Timer.
EventletScheduler - based on eventlet.spawn_after.
GeventScheduler - based on gevent.spawn_later.

@akatashev akatashev marked this pull request as draft May 16, 2020 22:13
@akatashev akatashev marked this pull request as ready for review May 18, 2020 23:34
@akatashev
Copy link
Author

I believe, that's possibly a typo:

self.event.wait()

Neither of EventletEvent and eventlet.event.Eventlet has event attribute. So, probably, it should be self.wait() or super().wait().
Didn't try to "fix" it, because I'm not sure what exactly it should be.

Meanwhile flake8 is angry about _future.pyi that I didn't touch. Other checks pass.

@codecov
Copy link

codecov bot commented May 22, 2020

Codecov Report

Merging #95 into develop will increase coverage by 0.51%.
The diff coverage is 99.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #95      +/-   ##
===========================================
+ Coverage    95.37%   95.88%   +0.51%     
===========================================
  Files           13       14       +1     
  Lines          562      656      +94     
===========================================
+ Hits           536      629      +93     
- Misses          26       27       +1     
Impacted Files Coverage Δ
pykka/_scheduler.py 98.68% <98.68%> (ø)
pykka/_threading.py 96.61% <100.00%> (+0.38%) ⬆️
pykka/eventlet.py 91.17% <100.00%> (+0.85%) ⬆️
pykka/gevent.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c4c9a7...4863fc3. Read the comment docs.

@fjarri fjarri mentioned this pull request Nov 3, 2020
@djmattyg007
Copy link

The code in this PR that calls time.time() really should be updated to use a monotonic timer.

Base automatically changed from develop to main March 6, 2021 18:16
@lostcontrol
Copy link

@jodal any plan on having this merged at some point? This is also a much needed feature. There are workarounds but would be nice to have something in Pykka directly.

Still, it would be nice to have it playing nicely with proxies also.

@akatashev
Copy link
Author

@lostcontrol I believe, this PR is extremely outdated, Pykka 3 is pretty different to the version that I had, when I was working this PR. It would need heavy refactoring. And there is a number of possible improvements, like the one, that @djmattyg007 mentioned. I'd be happy to refactor it, improve it and resolve all the conflicts, but I am not sure whether it makes any sense since there is no reaction from @jodal on any of opened PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants