jayyyyqwq opened a new pull request, #40925:
URL: https://github.com/apache/beam/pull/40925
`SlidingWindows.__init__` in the Python SDK checks that `size` is positive
but never checks `period`.
With `period=0` the constructor fails on `Timestamp.of(offset) % period`
with `ZeroDivisionError: integer modulo by zero` (window.py:479). With a
negative period, for example `SlidingWindows(size=10, period=-5)`, it
constructs fine and `assign()` returns `[]`, because the `range(...)` in
`assign` steps the wrong way. A pipeline using it drops every element without
any error.
`FixedWindows(size=0)` and `Sessions(gap_size=0)` already raise
`ValueError('The size parameter must be strictly positive.')`, and the Java
`SlidingWindows` rejects this through its `0 <= offset < period` check. I added
the matching check for `period` right after the `size` check:
if period <= 0:
raise ValueError('The period parameter must be strictly positive.')
Testing: I added `test_sliding_windows_non_positive_period` to
`window_test.py`. Before the change it fails with `ZeroDivisionError: integer
modulo by zero` at window.py:479. It also asserts that `FixedWindows(size=0)`
raises `ValueError`, which passes before and after. After the change
`window_test.py` passes (20 passed), and the `sliding or window` tests in
`sideinputs_test.py` and `trigger_manager_test.py` pass (12 passed). All
callers in the repo pass a positive literal period. I did not run
`combiners_test.py`, `util_test.py` or `pipeline_test.py` locally, since my
venv lacks `pyhamcrest` and `mock`, and I did not run the full CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
