houqp commented on a change in pull request #8545:
URL: https://github.com/apache/airflow/pull/8545#discussion_r421708159
##########
File path: airflow/models/baseoperator.py
##########
@@ -392,10 +442,79 @@ def __init__(
% (self.task_id, dag.dag_id))
self.sla = sla
self.execution_timeout = execution_timeout
+
+ # Warn about use of the deprecated SLA parameter
+ if sla and expected_finish:
+ warnings.warn(
+ "Both sla and expected_finish provided as task "
+ "parameters to {}; using expected_finish and ignoring "
+ "deprecated sla parameter.".format(self),
+ category=PendingDeprecationWarning
+ )
+ elif sla:
+ warnings.warn(
+ "sla is deprecated as a task parameter for {}; converting to "
+ "expected_finish instead.".format(self),
+ category=PendingDeprecationWarning
+ )
+ expected_finish = sla
+
+ # Set SLA parameters, batching invalid type messages into a
+ # single exception.
+ sla_param_errs: List = []
+ if expected_duration and not isinstance(expected_duration, timedelta):
Review comment:
it should be enforced in the CI pipeline at build time. If you tested
mypy and it's not complaining about it, then it's because mypy is still an
evolving project, so it can't catch all the type errors yet, but it's safe to
assume that it will eventually catch up.
i wouldn't worry too much about these edge-cases to be honest. we are not
doing runtime type check anywhere else where type hint is defined in the code
base, so it's a little bit odd to leave this one as a special case. On top of
that, this runtime type check will result in an exception. The end result is
the same as without this check because they will all lead to unrecoverable
crashes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
