Timer: Remove periodic() and timeout()
#264
Conversation
github-actions
bot
added
part:docs
src/frequenz/channels/timer.py
Outdated
| This quick start is provided to have a quick feeling of how to use this module, but | ||
| it is extremely important to understand how timers behave when they are delayed. | ||
|
|
||
| We recommned emphatically to read about [missed ticks and |
| @@ -27,7 +27,7 @@ async def test_file_watcher(tmp_path: pathlib.Path) -> None: | |||
| expected_number_of_writes = 3 | |||
|
|
|||
| file_watcher = FileWatcher(paths=[str(tmp_path)]) | |||
| timer = Timer.timeout(timedelta(seconds=0.1)) | |||
| timer = Timer(timedelta(seconds=0.1), SkipMissedAndDrift()) | |||
There was a problem hiding this comment.
do you think SkipMissedAndDrift should be an instance created in the channel package, so that we don't have to specify the () to create an instance of the thing?
This was a bit confusing for me the first time I did this, because I was expecting to pass just the policy, not an object of the policy that I had to create.
There was a problem hiding this comment.
The policies can have options, and SkipMissedAndDrift() have one (delay_tolerance). I think if we provide fixed instances with fixed options we'll eventually need another PR to remove them because they are still hiding the complexity :)
|
Although not strictly necessary, in this case it is easy to make all commits pass CI by just moving the first commit to near the end. |
True, but I thought review-wise It makes more sense to start with the important change first, and then adapt the code to deal with that change. I'm trying to prioritize easy of review when splitting the commits. I can move the commit if you prefer though... |
Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
Having the note at the end, just before the Missed ticks and drifting section starts doesn't make a lot of sense, it is better to warn the user before they start reading. Also change from a tip to an important information. Signed-off-by: Leandro Lucarella <[email protected]>
These names proved to be too confusing for users. It seems like having a short name that can convey all the intricacies of timers in the async world is very difficult, so we better stop trying to hide the complexity and force users to read about and use missed tick policies explicitly. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
llucax
force-pushed
the
rm-timer-ctors
branch
from
8b156ea to
cbccb5c
Compare
|
Updated to fix the typo and move the removal from the code to the end. |
|
Enabled auto-merging. |
The names are just too confusing and we'll never find a name that can convey all the intricacies of timers in the async world, so it is better to just remove
periodic()andtimeout()and force users to pass the missing ticks policies manually.Fixes #253.