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 2020/10/13 01:30:49 UTC
[GitHub] [airflow] jmcarp opened a new pull request #11486: Annotate DagRun methods with return types.
jmcarp opened a new pull request #11486:
URL: https://github.com/apache/airflow/pull/11486
I was debugging an unrelated issue and noticed that some DagRun methods don't have return type annotations. This patch adds annotations for the methods that I thought could use them.
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #11486: Annotate DagRun methods with return types.
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #11486:
URL: https://github.com/apache/airflow/pull/11486#issuecomment-721445984
Can you please rebase your PR on latest Master since we have applied [Black](https://github.com/apache/airflow/commit/4e8f9cc8d02b29c325b8a5a76b4837671bdf5f68) and [PyUpgrade](https://github.com/apache/airflow/commit/8c42cf1b00c90f0d7f11b8a3a455381de8e003c5) on Master.
It will help if your squash your commits into single commit first so that there are less conflicts.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] kaxil merged pull request #11486: Annotate DagRun methods with return types.
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #11486:
URL: https://github.com/apache/airflow/pull/11486
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] jmcarp commented on pull request #11486: Annotate DagRun methods with return types.
Posted by GitBox <gi...@apache.org>.
jmcarp commented on pull request #11486:
URL: https://github.com/apache/airflow/pull/11486#issuecomment-736104394
Updated/rebased.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11486: Annotate DagRun methods with return types.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11486:
URL: https://github.com/apache/airflow/pull/11486#issuecomment-745983491
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 master 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #11486: Annotate DagRun methods with return types.
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11486:
URL: https://github.com/apache/airflow/pull/11486#discussion_r507626299
##########
File path: airflow/models/dagrun.py
##########
@@ -310,15 +315,13 @@ def get_task_instance(self, task_id: str, session: Session = None):
:param session: Sqlalchemy ORM Session
:type session: Session
"""
- ti = session.query(TI).filter(
+ return session.query(TI).filter(
TI.dag_id == self.dag_id,
TI.execution_date == self.execution_date,
TI.task_id == task_id
).first()
- return ti
-
- def get_dag(self):
+ def get_dag(self) -> 'DAG':
Review comment:
Since you have the `if TYPE_CHECKING` can this use the non-string form?
##########
File path: airflow/models/dagrun.py
##########
@@ -301,7 +306,7 @@ def get_task_instances(self, state=None, session=None):
return tis.all()
@provide_session
- def get_task_instance(self, task_id: str, session: Session = None):
+ def get_task_instance(self, task_id: str, session: Session = None) -> TI:
Review comment:
I _think_ this might be `Optional[TI]`
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] jmcarp commented on pull request #11486: Annotate DagRun methods with return types.
Posted by GitBox <gi...@apache.org>.
jmcarp commented on pull request #11486:
URL: https://github.com/apache/airflow/pull/11486#issuecomment-745704927
Rebased again. @ashb, did I miss anything?
----------------------------------------------------------------
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:
users@infra.apache.org