You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/12/02 23:51:09 UTC
[GitHub] [airflow] ashb opened a new pull request #20000: Fix many of the mypy typing issues in airflow.models.dag
ashb opened a new pull request #20000:
URL: https://github.com/apache/airflow/pull/20000
And to fix these, I needed to fix a few other mistakes that are
used/called by DAG's methods.
Relates to https://github.com/apache/airflow/issues/19891 - it doesn't completely fix any packages, but this change was getting larger.
/cc @khalidmammadov
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #20000: Improve handling edge-cases in airlfow.models by applying mypy
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r763868344
##########
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:
Added note to CONTRIBUTING doc. Didn't seem worth it to add a pre-commit check for it right now -- if it starts slipping through then we can add one at a later date.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #20000: Fix many of the mypy typing issues in airflow.models
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r763083954
##########
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:
And shorter
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #20000: Fix many of the mypy typing issues in airflow.models
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r763657368
##########
File path: airflow/models/taskinstance.py
##########
@@ -561,7 +579,7 @@ def command_as_list(
def generate_command(
dag_id: str,
task_id: str,
- run_id: str = None,
+ run_id: str,
Review comment:
For other reviewers wondering, this method returns a command list (for e.g. subprocess), and passing in a None would not work, so all existing usages of this function should already be passing in a `run_id`, making this change safe.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #20000: Fix many of the mypy typing issues in airflow.models.dag
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r762161610
##########
File path: airflow/utils/timezone.py
##########
@@ -175,6 +175,16 @@ def parse(string: str, timezone=None) -> DateTime:
return pendulum.parse(string, tz=timezone or TIMEZONE, strict=False) # type: ignore
+@overload
+def coerce_datetime(v: None) -> None:
+ ...
+
+
+@overload
+def coerce_datetime(v: Union[dt.datetime, DateTime]) -> DateTime:
+ ...
Review comment:
Ah good to know - I wasn't aware of that.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #20000: Fix many of the mypy typing issues in airflow.models.dag
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#issuecomment-985110025
AH NOOO!!!!!! I MISSED THE #2000 one! Bummer!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #20000: Fix many of the mypy typing issues in airflow.models.dag
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r762161039
##########
File path: airflow/models/dagrun.py
##########
@@ -138,7 +138,7 @@ def __init__(
self,
dag_id: Optional[str] = None,
run_id: Optional[str] = None,
- queued_at: Optional[datetime] = __NO_VALUE,
+ queued_at: Optional[datetime] = __NO_VALUE, # type: ignore
Review comment:
This can benefit from the new `ArgNotSet` class in `airflow.utils.types`.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #20000: Fix many of the mypy typing issues in airflow.models
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r763071994
##########
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:
Fine for me. It's just an annoying feeling that some of those edge cases might fix some "real errors" :) and just indication in the commit message that it could improve things might be good for commit message in this case:
"Improve handling edge-cases in airlfow models by applying mypy suggestions", maybe :)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #20000: Fix many of the mypy typing issues in airflow.models
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r763083688
##########
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:
YEah. I also could not find a better way. `assert` is much more straightforward I think.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #20000: Fix many of the mypy typing issues in airflow.models.dag
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#issuecomment-985110025
AH NOOO!!!!!! I MISSED THE #20000 one! Bummer!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #20000: Fix many of the mypy typing issues in airflow.models.dag
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#issuecomment-985279083
It’s probably because Pendulum invents its own timezone class (which is fine) and expects to only receive that timezone class in its own datetime class (which is not fine). I’ll take a look.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #20000: Improve handling edge-cases in airlfow.models by applying mypy
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#issuecomment-988031968
Green build https://github.com/apache/airflow/actions/runs/1549766290 -- rebasing + merging.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #20000: Improve handling edge-cases in airlfow.models by applying mypy
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#issuecomment-988019541
Conflict is only in the import section:
```
<<<<<<< models-typing-partial
from airflow.utils.session import NEW_SESSION, create_session, provide_session
=======
from airflow.utils.retries import run_with_db_retries
from airflow.utils.session import create_session, provide_session
>>>>>>> main
```
so once these tests pass I'll fix conflict then merge without waiting for another test run.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #20000: Fix many of the mypy typing issues in airflow.models
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r762174878
##########
File path: airflow/models/skipmixin.py
##########
@@ -114,10 +114,7 @@ def skip(
session.commit()
# SkipMixin may not necessarily have a task_id attribute. Only store to XCom if one is available.
- try:
- task_id = self.task_id
- except AttributeError:
- task_id = None
+ task_id: Optional[str] = getattr(self, 'task_id')
Review comment:
fixed in f77d88edf
##########
File path: airflow/utils/timezone.py
##########
@@ -175,6 +175,16 @@ def parse(string: str, timezone=None) -> DateTime:
return pendulum.parse(string, tz=timezone or TIMEZONE, strict=False) # type: ignore
+@overload
+def coerce_datetime(v: None) -> None:
+ ...
+
+
+@overload
+def coerce_datetime(v: Union[dt.datetime, DateTime]) -> DateTime:
+ ...
Review comment:
fixed in f77d88edf
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #20000: Fix many of the mypy typing issues in airflow.models
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r763067505
##########
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:
Yeah, I wans't to happy about doing this, hence the comment. Let me take another look at this and see if I can avoid an assert at all.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #20000: Fix many of the mypy typing issues in airflow.models
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r763076763
##########
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:
K, I think it's this, or a `raise RuntimeError`.
Anyone have a preference?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #20000: Improve handling edge-cases in airlfow.models by applying mypy
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r763978634
##########
File path: airflow/models/dagrun.py
##########
@@ -96,7 +94,7 @@ class DagRun(Base, LoggingMixin):
last_scheduling_decision = Column(UtcDateTime)
dag_hash = Column(String(32))
- dag = None
+ dag: "Optional[DAG]" = None
Review comment:
This is what is causing the docs build to fail.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb merged pull request #20000: Improve handling edge-cases in airlfow.models by applying mypy
Posted by GitBox <gi...@apache.org>.
ashb merged pull request #20000:
URL: https://github.com/apache/airflow/pull/20000
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #20000: Improve handling edge-cases in airlfow.models by applying mypy
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r763989904
##########
File path: airflow/models/dagrun.py
##########
@@ -96,7 +94,7 @@ class DagRun(Base, LoggingMixin):
last_scheduling_decision = Column(UtcDateTime)
dag_hash = Column(String(32))
- dag = None
+ dag: "Optional[DAG]" = None
Review comment:
I have a fix for it -- so we can merge this and spend a bit more time looking at the sphinx upgrade.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #20000: Fix many of the mypy typing issues in airflow.models.dag
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#issuecomment-985094871
@uranusjr There were a couple of odd ones related to the timetable that I couldn't decipher:
```
airflow/models/dag.py:102: error: Variable "airflow.models.dag.ScheduleIntervalArgNotSet" is not valid as a type
ScheduleIntervalArg = Union[ScheduleInterval, None, Type[ScheduleIntervalArgNotSet]]
^
airflow/models/dag.py:102: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
airflow/models/dag.py:159: error: Argument 2 to "CronDataIntervalTimetable" has incompatible type "tzinfo"; expected "Timezone"
return CronDataIntervalTimetable(interval, timezone)
^
airflow/models/dag.py:428: error: Incompatible types in assignment (expression has type "Union[Union[str, timedelta, relativedelta], None, Type[ScheduleIntervalArgNotSet?]]", variable has type "Union[str, timedelta, relativedelta]")
self.schedule_interval: ScheduleInterval = schedule_interval
^
airflow/models/dag.py:568: error: Argument 2 to "iter_dagrun_infos_between" of "DAG" has incompatible type "Optional[datetime]"; expected "DateTime"
for info in self.iter_dagrun_infos_between(start_date, end_date, align=False)
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #20000: Fix many of the mypy typing issues in airflow.models.dag
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r762160365
##########
File path: airflow/models/skipmixin.py
##########
@@ -114,10 +114,7 @@ def skip(
session.commit()
# SkipMixin may not necessarily have a task_id attribute. Only store to XCom if one is available.
- try:
- task_id = self.task_id
- except AttributeError:
- task_id = None
+ task_id: Optional[str] = getattr(self, 'task_id')
Review comment:
```suggestion
task_id: Optional[str] = getattr(self, 'task_id', None)
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #20000: Fix many of the mypy typing issues in airflow.models
Posted by GitBox <gi...@apache.org>.
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #20000: Fix many of the mypy typing issues in airflow.models
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#issuecomment-987599748
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #20000: Fix many of the mypy typing issues in airflow.models.dag
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r762159303
##########
File path: airflow/utils/timezone.py
##########
@@ -175,6 +175,16 @@ def parse(string: str, timezone=None) -> DateTime:
return pendulum.parse(string, tz=timezone or TIMEZONE, strict=False) # type: ignore
+@overload
+def coerce_datetime(v: None) -> None:
+ ...
+
+
+@overload
+def coerce_datetime(v: Union[dt.datetime, DateTime]) -> DateTime:
+ ...
Review comment:
`pendulum.DateTime` is a stdlib `datetime` subclass, so this is equivalent to
```suggestion
def coerce_datetime(v: dt.datetime) -> DateTime:
...
```
same goes for the original annotation; it can just be `Optional[dt.datetime]`.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #20000: Fix many of the mypy typing issues in airflow.models
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r762168756
##########
File path: airflow/models/dagrun.py
##########
@@ -138,7 +138,7 @@ def __init__(
self,
dag_id: Optional[str] = None,
run_id: Optional[str] = None,
- queued_at: Optional[datetime] = __NO_VALUE,
+ queued_at: Optional[datetime] = __NO_VALUE, # type: ignore
Review comment:
Nice. fixed in f77d88edf
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #20000: Fix many of the mypy typing issues in airflow.models
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r763070493
##########
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:
Oh I remember this one. This is actually a case of "it makes no difference at run time" since `ti.dag_run = dr` on the next line has the same end effect, but doing it this way makes mypy happier.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #20000: Fix many of the mypy typing issues in airflow.models
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r763068672
##########
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:
I don't _think_ this changes the behaviour, not for anything but obscure edge cases anyway.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #20000: Fix many of the mypy typing issues in airflow.models
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#discussion_r763084583
##########
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:
We could potentially do some automated check if "assert" is preceded by `if TYPE_CHECKING:` in pre-commit.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #20000: Improve handling edge-cases in airlfow.models by applying mypy
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #20000:
URL: https://github.com/apache/airflow/pull/20000#issuecomment-987904256
I might need to merge https://github.com/apache/airflow/pull/20079 first -- this is crashing in building docs for some reason.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org