potiuk commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r763049790
##########
File path: airflow/models/baseoperator.py
##########
@@ -1325,7 +1335,7 @@ def run(
execution_date=info.logical_date,
data_interval=info.data_interval,
)
- ti = TaskInstance(self, run_id=None)
+ ti = TaskInstance(self, run_id=dr.run_id)
Review comment:
Ditto. (separation out ? or changing commit subject).
##########
File path: airflow/models/baseoperator.py
##########
@@ -1301,8 +1305,13 @@ def run(
from airflow.models import DagRun
from airflow.utils.types import DagRunType
- start_date = start_date or self.start_date
- end_date = end_date or self.end_date or timezone.utcnow()
+ # Assertions for typing -- we need a dag, for this function, and when
we have a DAG we are
+ # _guaranteed_ to have start_date (else we couldn't have been added to
a DAG)
+ if TYPE_CHECKING:
+ assert self.start_date
Review comment:
Small NIT.
We have this
https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#don-t-use-asserts-outside-tests
II think this is good, smallesst way of handling it even if slightly
incorrect - it should be `assert self.start_date is not None` but in this case
it does not matter)
But technically speaking this statement violates the agreement (even though
it is TYPE_CHECKNG gurarded). Should we add it as an exception to CONTRIBUTING?
##########
File path: airflow/models/dag.py
##########
@@ -403,11 +403,12 @@ def __init__(
# set timezone from start_date
tz = None
if start_date and start_date.tzinfo:
- tz = pendulum.instance(start_date).timezone
+ tz = pendulum.instance(start_date, tz=start_date.tzinfo or
settings.TIMEZONE).timezone
Review comment:
Nice. Sounds like an actual improvement of behaviour (bug fix)? Should
we change the title of the PR/commit to make it clearer in the changelog (or
maybe separate those out)?
--
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]
