Fix async batching, timeout detection and time formatting errors - #55
Merged
Merged
Conversation
The timeout tests counted items against real sleeps and left 10 to 40 ms of slack. A sleep only promises to take at least as long as requested, so the counts changed on a busy machine and on a coarse clock: - Blocking sleeps that overshoot by 40 ms or more made timeout_generator yield one item fewer, in five test cases and in its doctest. - A 15.6 ms event loop clock resolution, the Windows default, let the 0.05 s timeout fire together with a 0.04 s sleep, so the detector tests stopped at 3 instead of 4. The sync tests and the doctest now run on a fake clock that only moves when it is slept on, and they check the requested sleeps as well. The total timeout tests advance the same clock. The per-item timeout tests yield without waiting and then stall for 10 s against a 0.05 s timeout. One test stays on the real clock and only checks what holds for any sleep accuracy. The fixtures are loaded from a conftest.py in the repository root so the doctests can use them, and the sdist ships that file.
test_aio_timeout_generator still counted items against real sleeps. The case with five sleeps of 0.06 s against a 0.3 s timeout ends one item short as soon as the sleeps run 15 ms late in total. It failed 3 of 25 runs on a busy machine, and fails every time when asyncio.sleep is made 20 ms late. The test now lets asyncio.sleep advance the fake clock. The default iterable test in test_lazy_imports uses the fake clock too, so its 0.05 s timeout cannot end the loop before the second item.
- The task that waits for the next item is cancelled and awaited when the consumer stops early, closes the batcher or is cancelled. It was left running, took the next item from the source and nobody received it. - A full batch starts a new interval. The old interval kept running, so the next item was flushed on its own. - A batch that is waiting is flushed when its interval ends. Each wakeup waited a full interval again, which could hold a batch for almost twice as long. - An iterator whose __anext__ returns a future is accepted.
aio_generator_timeout_detector: - A plain function works as on_timeout. Its result was awaited, which raised TypeError for anything but a coroutine function. - total_timeout ends the wait for an item that does not arrive. It was only checked between items, so a stalled generator outlived it. format_time: - Truncation to the precision is exact. With float seconds 1.0 % 0.1 is just under 0.1, so one second at 100 ms precision printed 0.9 seconds. - nan prints the placeholder, like infinity does. timeout_generator and aio_timeout_generator apply maximum_interval to the first sleep as well. timedelta_to_seconds divides the whole microseconds once, the way total_seconds() does. Whole seconds still give an int. The docstrings say that a timeout or maximum_interval of 0 means none, and the aio_timeout_generator text describes interval_multiplier instead of a parameter that does not exist.
With a negative step the stop is a lower bound, the way range reads it. acount(10, -2, stop=0) yielded nothing.
- sample logs through the logger of its module. The module-level logging.debug() installs a handler on the root logger when it has none, which made a later basicConfig() of the application a no-op. - listify keeps the name, docstring and signature of the function. - wraps_classmethod copies the annotations before it drops self. It changed the wrapped function, and on Python 3.14 the wrapper lost its own annotations. The listify docstring describes what allow_empty looks at.
… fix/time-and-async-helpers
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| await started.wait() | ||
| consumer.cancel() | ||
| with pytest.raises(asyncio.CancelledError): | ||
| await consumer |
| root because the doctests in ``python_utils`` need them as well. | ||
| """ | ||
|
|
||
| pytest_plugins: tuple[str, ...] = ('_python_utils_tests.clock',) |
| # A coroutine function hands back something to await, a | ||
| # plain function has already done its work by now. | ||
| if isinstance(result, collections.abc.Awaitable): | ||
| await result |
An adversarial pass compared the fixes on this branch with 4.0.1. Two of them changed more than the error they were for, so they are taken back. - abatcher waits a full interval per wakeup again and flushes when the clock has passed the interval. Waiting only for the rest of the interval gave a third to a half more batches on a steady stream. It also left an item in flight at nearly every yield, so that stopping early ended the source generator where 4.0.1 left it usable. - The detector checks total_timeout between items again. Bounding the wait ran every step of the wrapped generator in a new task, which lost its context variables and task-bound timeouts, could swallow a cancellation on Python 3.10 and 3.11, and no longer delivered an item that arrived just after the deadline. A clean bound needs asyncio.timeout, which Python 3.10 does not have. The docstring says what the total timeout covers. Kept, and adjusted: - maximum_interval limits the first sleep without changing the later ones. Clamping the interval itself shifted the whole sequence when the multiplier is below 1. - An error that the source raises while abatcher cancels its pending item is reported to the event loop instead of dropped.
- With nothing on its way the cleanup does nothing. An empty gather looks up an event loop, which failed when the garbage collector closed an unfinished batcher after its loop was gone. - A source that returns when it is cancelled is no longer reported to the event loop as a failure. Its item ends with StopAsyncIteration. The docstring says what stopping early does to the source.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found by an adversarial pass over the modules that are unchanged since 4.0.1. Every fix started as a failing test. Only errors whose fix cannot break working code are in here. The rest is listed at the bottom for a decision.
The branch contains #53, so the test hooks pass on any machine. Merge #53 first, or merge this and #53 is included.
abatcher
batch_size=3,interval=10and an item every 4 seconds the batches were[0, 1, 2],[3],[4, 5, 6],[7].__anext__returns a future is accepted.aio_generator_timeout_detector
A plain function works as
on_timeout. It raisedTypeError: 'NoneType' object can't be awaitedafter the callback had run.format_time and timedelta_to_seconds
format_time(1, timedelta(milliseconds=100))0:00:00.9000000:00:01format_time(float('nan'))ValueError--:--:--timedelta_to_seconds(timedelta(microseconds=-1))-9.99993e-07-1e-06Output for the default precision is identical to 4.0.1 on 200000 random inputs across four time zones.
Smaller fixes
timeout_generatorandaio_timeout_generatorlimit the first sleep tomaximum_intervaltoo. The later sleeps are unchanged.acount(10, -2, stop=0)counts down. It yielded nothing.samplelogs through its module logger. The module-levellogging.debug()installed a root handler, which made a laterbasicConfig()of the application a no-op.listifykeeps the name, docstring and signature of the function.wraps_classmethodno longer changes the annotations of the wrapped function, and keeps the wrapper's own on Python 3.14.Docstrings corrected, no behaviour change
timeoutormaximum_intervalof0means none.total_timeoutis checked between items and does not end the wait for an item that does not arrive.aio_timeout_generatordescribed aninterval_exponentthat does not exist.listify(allow_empty=False)only rejects aNoneresult.Compatibility
A second adversarial pass compared this branch with 4.0.1 over 540
abatcherscenarios and 385 detector scenarios. It found that two fixes in the first version changed more than their error, so both were taken back:abatcherbatch exactly when its interval ends gave a third to a half more batches on a steady stream.total_timeoutran each step of the wrapped generator in a new task, which loses context variables, and dropped an item that 4.0.1 still delivered.With those reverted, the released 4.0.1 tests for
abatcher,acount, the detector and the decorators pass unchanged against this branch.What working code can still notice:
abatcherloop while an item is on its way, the source generator receivesCancelledErrorat its pending await. In 4.0.1 a leaked task took that item. A source with nothing on its way stays usable, as before.timedelta_to_secondsreturns a slightly different float for some non-whole durations, equal tototal_seconds().samplehas the logger namepython_utils.decoratorsinstead ofroot.listifyreports its own__name__instead of__listify.acountwith a negative step and a stop above the start yields nothing, likerange. It ignored the stop and counted down forever.format_timewith aprecisionthat is not a timedelta raisesTypeErrorinstead ofAttributeError, and raises nothing forNoneand plain dates, which never use the precision.Found and left alone
These change what working code sees, so they need a decision:
total_timeoutdoes not end the wait for a stalled item. A clean fix needsasyncio.timeout, which Python 3.10 does not have.abatcherbatch can wait almost two intervals before it is flushed.TimeoutErrorraised inside the wrapped generator as its own timeout.format_timeprints a timezone-aware datetime as local wall time without the offset, and truncates on UTC boundaries.format_time(datetime.min)andformat_time(datetime.max)raiseValueError.batcherandabatcheraccept abatch_sizeof 0 or below and buffer the whole stream.acountsleeps once more after its last value. An existing test asserts that count.