sadpandajoe commented on code in PR #43782:
URL: https://github.com/apache/superset/pull/43782#discussion_r3908700100
##########
superset/tasks/decorators.py:
##########
@@ -251,8 +251,9 @@ def _merge_options(self, override_options: TaskOptions |
None) -> TaskOptions:
"""
Merge decorator defaults with call-time overrides.
- Call-time options take precedence over decorator defaults.
- For timeout, an explicit None in TaskOptions disables the decorator
timeout.
+ Call-time options take precedence over decorator defaults. A call-time
+ ``timeout`` overrides only when set to a concrete value; ``None``
inherits
+ the decorator's timeout.
Review Comment:
The updated docstring says `timeout=None` inherits the decorator timeout,
but the nearby merge comment still says explicit `None` is used. That leaves
opposite guidance at the decision point and can invite a future change that
disables timeouts for callers that omit `timeout`; could the inline comment be
corrected too?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
