pankajkoti commented on code in PR #31712:
URL: https://github.com/apache/airflow/pull/31712#discussion_r1233606897
##########
airflow/config_templates/default_airflow.cfg:
##########
@@ -692,6 +692,9 @@ password =
# The default owner assigned to each new operator, unless
# provided explicitly or passed via ``default_args``
default_owner = airflow
+
+# The default value of attribute "deferrable" in BaseDeferrableOperator.
+default_deferrable = false
Review Comment:
```suggestion
default_deferrable = False
```
The boolean values in this file seem to be in Title case
##########
airflow/providers/amazon/aws/operators/batch.py:
##########
@@ -143,7 +151,7 @@ def __init__(
region_name: str | None = None,
tags: dict | None = None,
wait_for_completion: bool = True,
- deferrable: bool = False,
+ deferrable: bool = DEFAULT_DEFERRABLE,
Review Comment:
Not suggesting a change for this PR, but, wondering if we need to add a
pre-commit hook to check that the default value for this param is set to
DEFAULT_DEFERRABLE so that someone who is not aware of this change knows about
it when they add new deferrable support to other operators/sensors.?
##########
airflow/models/baseoperator.py:
##########
@@ -1874,3 +1877,21 @@ def get_link(self, operator: BaseOperator, *, ti_key:
TaskInstanceKey) -> str:
:param ti_key: TaskInstance ID to return link for.
:return: link to external system
"""
+
+
+class BaseDeferrableOperator(BaseOperator):
+ """Abstract base class for deferrable operators.
Review Comment:
This does not seem to be a true Abstract class as it's not extending
`abc.ABC` and users might be able to create instances of this class. Can we
recheck the wording here please based on whether we want to allow the creation
of instances for this class or not? Or extend it from `abc.ABC`? WDYT?
--
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]
