anxkhn commented on PR #69118: URL: https://github.com/apache/airflow/pull/69118#issuecomment-4853586394
Thanks for the thorough review, all points addressed: 1. Kept the warning: the eventual-consistency note is now a `.. warning::` directive with your wording. 2. Shortened the `empty_confirmations` param docstring to the concise version (it states the default of 2). 3. Made `poke`'s return explicit (`if ...: return True` / `return False`). 4. Moved the eventual-consistency explanation out of the trigger class docstring and into a comment inside `run()`, next to the confirmation-counting logic. 5. Parametrized the trigger test over `empty_confirmations` 2 and 3, asserting it waits for exactly that many consecutive empty polls before yielding success. 6. Parametrized the sensor test the same way. On the broader semantics point: you are right that the sensor now reports the stable state after N confirmations instead of the first observed reading, which can delay success by up to one `poke_interval` versus earlier releases. That is intentional (it closes the false-empty window) and is now called out in the `.. warning::` so it is not surprising. Ran ruff (check + format), mypy, and both edited test modules locally, all green. -- 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]
