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/03/27 00:45:54 UTC
[GitHub] [airflow] mik-laj opened a new pull request #7900: Convert
properties with query to real methods
mik-laj opened a new pull request #7900: Convert properties with query to real methods
URL: https://github.com/apache/airflow/pull/7900
These queries are very difficult to see during the review. This can cause serious performance issues. If everything will be explicitly expressed, it is easier to see suboptimal database queries.
Example: https://github.com/apache/airflow/pull/7476
---
Issue link: WILL BE INSERTED BY [boring-cyborg](https://github.com/kaxil/boring-cyborg)
Make sure to mark the boxes below before creating PR: [x]
- [X] Description above provides context of the change
- [X] Unit tests coverage for changes (not needed for documentation changes)
- [X] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [X] Relevant documentation is updated including usage instructions.
- [X] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
---
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).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #7900: Convert properties with
query to real methods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #7900: Convert properties with query to real methods
URL: https://github.com/apache/airflow/pull/7900#issuecomment-604873922
@potiuk Good point. I will add backward compatibility.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk merged pull request #7900: Convert properties with
query to real methods
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #7900: Convert properties with query to real methods
URL: https://github.com/apache/airflow/pull/7900
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] feluelle commented on a change in pull request #7900:
Convert properties with query to real methods
Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #7900: Convert properties with query to real methods
URL: https://github.com/apache/airflow/pull/7900#discussion_r399343659
##########
File path: airflow/www/views.py
##########
@@ -93,7 +93,7 @@ def get_date_time_num_runs_dag_runs_form_data(request, session, dag):
if dttm:
dttm = timezone.parse(dttm)
else:
- dttm = dag.latest_execution_date or timezone.utcnow()
+ dttm = dag.get_latest_execution_date() or timezone.utcnow()
Review comment:
```suggestion
dttm = dag.get_latest_execution_date(session) or timezone.utcnow()
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] feluelle commented on a change in pull request #7900:
Convert properties with query to real methods
Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #7900: Convert properties with query to real methods
URL: https://github.com/apache/airflow/pull/7900#discussion_r399344592
##########
File path: airflow/www/views.py
##########
@@ -93,7 +93,7 @@ def get_date_time_num_runs_dag_runs_form_data(request, session, dag):
if dttm:
dttm = timezone.parse(dttm)
else:
- dttm = dag.latest_execution_date or timezone.utcnow()
+ dttm = dag.get_latest_execution_date() or timezone.utcnow()
Review comment:
There are more of it where we can just pass the session which is better than recreating it, don't you 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] feluelle commented on a change in pull request #7900:
Convert properties with query to real methods
Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #7900: Convert properties with query to real methods
URL: https://github.com/apache/airflow/pull/7900#discussion_r402376564
##########
File path: airflow/models/taskinstance.py
##########
@@ -596,28 +602,80 @@ def _get_previous_ti(
return None
@property
- def previous_ti(self) -> Optional['TaskInstance']:
- """The task instance for the task that ran before this task instance."""
- return self._get_previous_ti()
+ def previous_ti(self):
+ """
+ This attribute is deprecated.
+ Please use `airflow.models.taskinstance.TaskInstance.get_previous_ti` method.
+ """
+ warnings.warn(
+ """
+ This attribute is deprecated.
+ Please use `airflow.models.taskinstance.TaskInstance.get_previous_ti` method.
+ """,
+ DeprecationWarning,
+ stacklevel=2,
+ )
+ return self.get_latest_execution_date()
@property
def previous_ti_success(self) -> Optional['TaskInstance']:
- """The ti from prior succesful dag run for this task, by execution date."""
+ """
+ This attribute is deprecated.
+ Please use `airflow.models.taskinstance.TaskInstance.get_previous_ti` method.
+ """
+ warnings.warn(
+ """
+ This attribute is deprecated.
+ Please use `airflow.models.taskinstance.TaskInstance.get_previous_ti` method.
+ """,
+ DeprecationWarning,
+ stacklevel=2,
+ )
return self._get_previous_ti(state=State.SUCCESS)
- @property
- def previous_execution_date_success(self) -> Optional[pendulum.datetime]:
- """The execution date from property previous_ti_success."""
- self.log.debug("previous_execution_date_success was called")
- prev_ti = self._get_previous_ti(state=State.SUCCESS)
+ def get_previous_execution_date(
+ self,
+ state: Optional[str] = None,
+ session: Session = None,
+ ) -> Optional[pendulum.datetime]:
+ """
+ The execution date from property previous_ti_success.
+
+ :param state: If passed, it only take into account instances of a specific state.
+ """
+ self.log.debug("previous_execution_date was called")
+ prev_ti = self.get_previous_ti(state=state, session=session)
return prev_ti and prev_ti.execution_date
+ def get_previous_start_date(
+ self,
+ state: Optional[str] = None,
+ session: Session = None
+ ) -> Optional[pendulum.datetime]:
+ """
+ The start date from property previous_ti_success.
+
+ :param state: If passed, it only take into account instances of a specific state.
+ """
+ self.log.debug("previous_start_datewas called")
Review comment:
```suggestion
self.log.debug("previous_start_date was called")
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] feluelle commented on a change in pull request #7900:
Convert properties with query to real methods
Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #7900: Convert properties with query to real methods
URL: https://github.com/apache/airflow/pull/7900#discussion_r399342749
##########
File path: airflow/ti_deps/deps/prev_dagrun_dep.py
##########
@@ -64,7 +64,7 @@ def _get_dep_statuses(self, ti, session, dep_context):
reason="This task instance was the first task instance for its task.")
return
- previous_ti = ti.previous_ti
+ previous_ti = ti.get_previous_ti()
Review comment:
```suggestion
previous_ti = ti.get_previous_ti(session=session)
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] feluelle commented on a change in pull request #7900:
Convert properties with query to real methods
Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #7900: Convert properties with query to real methods
URL: https://github.com/apache/airflow/pull/7900#discussion_r399342406
##########
File path: airflow/ti_deps/deps/dag_unpaused_dep.py
##########
@@ -29,6 +29,6 @@ class DagUnpausedDep(BaseTIDep):
@provide_session
def _get_dep_statuses(self, ti, session, dep_context):
- if ti.task.dag.is_paused:
+ if ti.task.dag.get_is_paused():
Review comment:
```suggestion
if ti.task.dag.get_is_paused(session):
```
----------------------------------------------------------------
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
With regards,
Apache Git Services