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

AsyncScheduler: Ability to schedule сoroutines #357

Open
wants to merge 30 commits into
base: master
Choose a base branch
from

Conversation

pirroman
Copy link

Hey. Added the ability to create a scheduler for asynchronous tasks. The implementation of AsyncScheduler does not affect the existing functionality in any way and backward compatibility is fully supported. Even on python 2.7 the library also continue works. The new functionality has been covered with tests.

P.S. I plan to use this tool in my projects, so the asynchronous part will be developed and maintained by me.

@grmozhaev
Copy link

Great job! Hope to see this PR merged soon.

@coveralls
Copy link

coveralls commented Mar 19, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 8e1c263 on pirroman:async_scheduler into 705f373 on dbader:master.

@ashwinrz
Copy link

ashwinrz commented Apr 3, 2020

Awesome! Can we get this merged?

@aekasitt
Copy link

Bump

@davenewham
Copy link

bump again. This is a great feature. I pulled the branch and it works perfectly :)

@pirroman
Copy link
Author

@davenewham Thanks! You can fork from my fork ;)

Copy link

@07pepa 07pepa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe will be smart to add python 3.9 and test it also

setup.py Outdated Show resolved Hide resolved
schedule/__init__.py Outdated Show resolved Hide resolved
@07pepa
Copy link

07pepa commented Oct 14, 2020

I have related pull request #391 and i noticed yours now.

But you solution for running corutines is much better....mine was mostly concerned about handling os signals fast (even with long sleeps/waits between runs of run_pending)

i think we should join our efforts since you will eventualy have to write about pitfalls of asyncio and fact it will run concurenly all sechuded jobs... so control/data race condition may emerge

@pirroman
Copy link
Author

pirroman commented Jan 4, 2021

@07pepa, Hello! I'm so sorry for so long to respond. I am always in favor of cooperation. I suggest you make a PR to my fork. I promise to review it soon as possible.

@07pepa
Copy link

07pepa commented Jan 4, 2021

@07pepa, Hello! I'm so sorry for so long to respond. I am always in favor of cooperation. I suggest you make a PR to my fork. I promise to review it soon as possible.

i will try to get it to it in next few weeks

@pirroman
Copy link
Author

pirroman commented Jan 5, 2021

Hi @SijmenHuizenga!

Assuming we drop python 2 support (which we should probably do before next release) we could just add the async methods to the main scheduler. In that case we wouldn't need to refactor and keep everything in a single scheduler class. Also, we could keep using the same testsuite for all python versions. I'm not entirely sure what the drawbacks of extra async functions in the main scheduler would be, ideas are welcome 😄

I don't have a problem with dropping support for python2 and python3.5. Then it would be easy to merge the tests and library support would be greatly simplified. But it doesn't have to be done right now, I wouldn't want to do that work as part of this request. Besides, while this request is pending, I'm getting conflicts( You can make an issue and I will do it in the next request.

schedule.every().day.do_async(job)

There is no sense in explicitly dividing tasks into synchronous and asynchronous. Asynchronous scheduler can handle all types. So the problem is with the methods: run_pending and run_all. Is it worth moving these methods to the current Scheduler? It seems to me not, because the delimitation will become less strict. Now, having an asynchronous scheduler you have no possibility to call synchronous methods run_pending and run_all, after merging this possibility will appear. All in all, for users the library will turn into mush. That' s my opinion.

Simplicity includes having a single source file and having few classes.

I don't agree that simplicity comes from having a single file. I thought that in this case simplicity meant ease to use and a low threshold to get into the library. I may have misinterpreted the concept of simplicity of this library. But I agree to discuss the possibility of simplicity by separating classes into separate files and by designing a simple and clear architecture. In my opinion it is easier to navigate the code base of the library this way, but I also respect your opinion on this.

By adding the AsyncScheduler class, we are introducing a new concept to users: the scheduler type (the normal scheduler vs async scheduler). At this moment the extra scheduler is a small superset of features. But it opens the door for more types of schedulers... Next people will be building BackgroundScheduler and ThreadedScheduler, ...

Here I agree and disagree. This change is due to the extension of the python language - it became possible to declare asynchronous functions(coroutines), and the lack of support for asynchronous functions and gave a lot of limitations of this library in the modern language. Those examples you described are BackgroundScheduler and ThreadedScheduler. The tasks that should be handled by these concepts could be handled by AsyncScheduler. So I don't see any obvious reason for additional concepts to appear until another type of function appears in the language 😄.

into its own library?

I honestly don't like that idea very much either. There would be a need for constant synchronization, and besides I have made changes to the organization of the code base, which would make any synchronization an unpleasant process. I propose to consider this as an extreme case.

P.S. Thanks for the feedback. I hope that we will soon come to a common solution that satisfies everyone! I am ready to participate in further support of my solution. So if new issues are born in view of our discussion, I am ready to do it.

@SijmenHuizenga
Copy link
Collaborator

Thanks a lot for the detailed response! I will take some time to experiment and get my thoughts together. But first I will be working on the 1.0.0 release as detailed in #412. After that I will be back here :)

@SijmenHuizenga
Copy link
Collaborator

As an experiment I've implemented async support by adding run_pending_async and run_all_async to the main scheduler. See #438. I would love to hear what you think about supporting coroutines in such a way @pirroman

@ndbeals
Copy link

ndbeals commented Sep 14, 2021

Any news regarding the progress of this?

@pirroman
Copy link
Author

@SijmenHuizenga @ndbeals I apologize for this looong delay. I promise I'll get back to progress this weekend.

@pirroman
Copy link
Author

@SijmenHuizenga Honestly, I like my version more. It is stricter and more structured. That is, the explicit creation of an asynchronous scheduler in the code will explicitly talk about the ability to handle asynchronous tasks, while combining asynchronous and synchronous interfaces creates more risks of using synchronous interface on asynchronous tasks.

In the recent changes I went a little further and used inheritance only to share common functionality. Interface classes are no longer divided, thought that one interface returning different types is bad.

@gaby
Copy link

gaby commented Sep 28, 2021

@dbader Can we get this merge? PR has been open for almost 2 years!

@grantmagdanz
Copy link

I'm also looking for a way to schedule async tasks. Would love to get an update on if / when this might be merged!

@gaby
Copy link

gaby commented Nov 4, 2021

@dbader @SijmenHuizenga Friendly bump

@Kayden-lolasery
Copy link

It'll be nice if a note was placed in the README indicating no support for this library is being provided any longer

@gaby
Copy link

gaby commented Sep 19, 2022

The library had commits this year, but for some reason the developers don't answer on this PR

@NomadicDeveloper22
Copy link

@dbader not sure if this PR is just flying under the radar on your github notifications, but this seriously needs merged

@Dorbmon
Copy link

Dorbmon commented Aug 25, 2024

Hi, is there anything new about this? 4 years pass :)

@gaby
Copy link

gaby commented Aug 25, 2024

@Dorbmon One day

@Dorbmon
Copy link

Dorbmon commented Aug 25, 2024

@gaby sad :(

@gaby
Copy link

gaby commented Aug 25, 2024

Hello @SijmenHuizenga @dbader @connorskees Any chance of getting this merged? Having async support would be very helpful for this library.

Thanks!

@kulame
Copy link

kulame commented Nov 7, 2024

How is the progress? Are there other asynchronous scheduling libraries available as alternatives?

@gaby
Copy link

gaby commented Nov 7, 2024

@SijmenHuizenga Friendly ping

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

Successfully merging this pull request may close these issues.