-
Notifications
You must be signed in to change notification settings - Fork 965
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
Added documentation regarding corutines #391
Conversation
it looks like coverage is buggy... |
i decided it is not as dependend on #357 and can be fast forwarded |
@SijmenHuizenga can we Merge this?... any objections are welcomed :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this pr! I learnt some things from it 😄
The goal of the schedule documentation is to teach people how to use the schedule library. We are not in the business of explaining how asyncio works or state the advantages of using coroutines.
At the moment the code and text has too many details that are interesting, but not relevant to the schedule docs. I've added some comments to explain my thinking. Let me know if anything is unclear.
During reading and testing I have updated the page to how I see it. I will submit those changes in a separate pr to your branch so that you can decide if you want to include those changes into this pr.
docs/async-support.rst
Outdated
This code will only run on Wednesday regular task and Sunday coroutine task. | ||
Nothing is running in between but code not hangs on system signals and exits loop (on one thread that is impossible without coroutines). | ||
|
||
.. code-block:: python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example code has many good things in it. I think if we split up the example in a few shorter examples it becomes easier to understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed
docs/faq.rst
Outdated
|
||
Does schedule support coroutines? | ||
------------------------------- | ||
:doc:`yes <async-support>` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:doc:`yes <async-support>` | |
Schedule does not accept coroutines as jobs directly. However, using using ``asyncio.create_task`` you able to schedule async jobs. See :doc:`this page <async-support>` for an example. |
docs/async-support.rst
Outdated
@@ -0,0 +1,92 @@ | |||
Nonblocking scheduling with coroutines | |||
====================================== | |||
Fist is good to explain what we mean by nonblocking scheduling. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's try to focus on how the schedule library interacts with the asyncio Python module. The concept 'non-blocking scheduling' is not so much equal to asyncio... One could achieve non-blocking job execution without asyncio and asyncio can (in some cases) sill be blocking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that is true!....
docs/async-support.rst
Outdated
|
||
DrawBacks | ||
---------- | ||
Only drawback is precision of asyncio.wait_for timeout. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate how this affects job scheduling?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well i saw some variance in my personal project (few seconds) and real amount of "sleep" is better known for time.sleep https://stackoverflow.com/questions/1133857/how-accurate-is-pythons-time-sleep
so job can be executed later then expected but that is also expected for time.sleep it is not exact
docs/async-support.rst
Outdated
global terminateEvent | ||
terminateEvent = asyncio.Event() #needs to run on asyncio loop | ||
|
||
def terminate(signum): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What we have here is a quite a generic signal handler. It seems like a good implementation, but it doesn't have much to do with the library. Therefore I don't think this should be part of the schedule
library documentation.
I also don't remember seeing any questions (in issues) regarding interrupting time.sleep. Apparently it works out-of-the-box for most people. If this changes I would be happy to help write a dedicated page on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
time sleep and asyncio sleep are fine on their own
but when you use your custom sigal handler they dont recive KeyboardInterrupt
and if you want your code to finish corectly you should use them
but in that case you must have something that can be interupted
and you can reference back to issue i created....#388
i think we should put this as seperate example
added documentation regarding scheduling task and corutines in non-blocking loop
@SijmenHuizenga it is better now? |
I'm sorry for making you wait this long on a response. The docs should focus on explaining how the schedule library works and how to use it. The examples about interrupting a loop and handling signals are out of scope. These examples mostly teach the reader about Python and asyncio without teaching much about the schedule library. These examples would fit very well in a blog post or in a Github gist, but not so much in the official docs. I just pushed the shortened examples and will merge this pr soon. Thank you for working on these examples, I learnt some new things and I expect many people will too. |
This closes #388