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/25 17:31:43 UTC
[GitHub] [airflow] olchas opened a new pull request #11843: Fix issue with moving end_date in elastic dag
olchas opened a new pull request #11843:
URL: https://github.com/apache/airflow/pull/11843
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
This PR addresses an issue in the elastic DAG with moving `end_date` when using "PERF_MAX_RUNS" environment variable. Because `end_date` is derived from `start_date`, and `start_date` being defined as `datetime.now() - parse_time_delta(START_DATE_ENV)` constantly changes, this causes `end_date` to move forward in time as well. While (from what I understand) the `start_date` is assigned to a DAG only once, the `end_date` is not, so it is constantly updated, which allows scheduler to create more DagRuns than specified by "PERF_MAX_RUNS" (it can be easily observed especially with small "PERF_SCHEDULE_INTERVAL" values like "1m").
I have added option to provide `start_date` as a date string via "PERF_START_DATE" environment variable, which overrides "PERF_START_AGO" and is required when "PERF_MAX_RUNS" is defined. I also had to change the "START_DATE" part of the `dag_id`, which becomes the `start_date` timestamp when "PERF_START_DATE" is defined. We cannot use timestamp when using "PERF_START_AGO", again due to moving nature of `start_date` (it would cause the constant stream of new DAGs).
**^ Add meaningful description above**
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] mik-laj commented on a change in pull request #11843: Fix issue with moving end_date in elastic dag
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #11843:
URL: https://github.com/apache/airflow/pull/11843#discussion_r554452022
##########
File path: tests/test_utils/perf/dags/elastic_dag.py
##########
@@ -51,6 +51,24 @@ def parse_time_delta(time_str: str):
return timedelta(**time_params) # type: ignore
+def parse_start_date():
+ """
+ Returns the start date for the elastic DAGs and string to be used as part of their ids.
+
+ :return Tuple[datetime.datetime, str]: A tuple of datetime.datetime object to be used
+ as a start_date and a string that should be used as part of the dag_id.
+ """
+
+ if "PERF_START_DATE" in os.environ:
Review comment:
It will be ignored without any explicit message to the user, which may cause problems. I prefer to add one more check than then diagnose non-obvious behavior.
----------------------------------------------------------------
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] olchas commented on a change in pull request #11843: Fix issue with moving end_date in elastic dag
Posted by GitBox <gi...@apache.org>.
olchas commented on a change in pull request #11843:
URL: https://github.com/apache/airflow/pull/11843#discussion_r554924174
##########
File path: tests/test_utils/perf/dags/elastic_dag.py
##########
@@ -51,6 +51,24 @@ def parse_time_delta(time_str: str):
return timedelta(**time_params) # type: ignore
+def parse_start_date():
+ """
+ Returns the start date for the elastic DAGs and string to be used as part of their ids.
+
+ :return Tuple[datetime.datetime, str]: A tuple of datetime.datetime object to be used
+ as a start_date and a string that should be used as part of the dag_id.
+ """
+
+ if "PERF_START_DATE" in os.environ:
Review comment:
I agree. Added raising exception.
----------------------------------------------------------------
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] closed pull request #11843: Fix issue with moving end_date in elastic dag
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #11843:
URL: https://github.com/apache/airflow/pull/11843
----------------------------------------------------------------
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] olchas commented on pull request #11843: Fix issue with moving end_date in elastic dag
Posted by GitBox <gi...@apache.org>.
olchas commented on pull request #11843:
URL: https://github.com/apache/airflow/pull/11843#issuecomment-751649107
@mik-laj I have rebased this PR, could you 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #11843: Fix issue with moving end_date in elastic dag
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #11843:
URL: https://github.com/apache/airflow/pull/11843#discussion_r554452022
##########
File path: tests/test_utils/perf/dags/elastic_dag.py
##########
@@ -51,6 +51,24 @@ def parse_time_delta(time_str: str):
return timedelta(**time_params) # type: ignore
+def parse_start_date():
+ """
+ Returns the start date for the elastic DAGs and string to be used as part of their ids.
+
+ :return Tuple[datetime.datetime, str]: A tuple of datetime.datetime object to be used
+ as a start_date and a string that should be used as part of the dag_id.
+ """
+
+ if "PERF_START_DATE" in os.environ:
Review comment:
It will be ignored without any explicit message to the user, which may cause problems. I prefer to add one more check than then diagnose non-obvious behavior.
----------------------------------------------------------------
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] olchas commented on a change in pull request #11843: Fix issue with moving end_date in elastic dag
Posted by GitBox <gi...@apache.org>.
olchas commented on a change in pull request #11843:
URL: https://github.com/apache/airflow/pull/11843#discussion_r554450379
##########
File path: tests/test_utils/perf/dags/elastic_dag.py
##########
@@ -51,6 +51,24 @@ def parse_time_delta(time_str: str):
return timedelta(**time_params) # type: ignore
+def parse_start_date():
+ """
+ Returns the start date for the elastic DAGs and string to be used as part of their ids.
+
+ :return Tuple[datetime.datetime, str]: A tuple of datetime.datetime object to be used
+ as a start_date and a string that should be used as part of the dag_id.
+ """
+
+ if "PERF_START_DATE" in os.environ:
Review comment:
Currently, if `PERF_START_DATE` is defined, then the value of `PERF_START_AGO` (if defined as well) will simply be ignored. Are you sure we should throw an exception if both of them are defined? They are mutually exclusive, but having both of them defined does not prevent parsing the dag file.
----------------------------------------------------------------
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 #11843: Fix issue with moving end_date in elastic dag
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11843:
URL: https://github.com/apache/airflow/pull/11843#issuecomment-787558810
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
----------------------------------------------------------------
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] olchas commented on pull request #11843: Fix issue with moving end_date in elastic dag
Posted by GitBox <gi...@apache.org>.
olchas commented on pull request #11843:
URL: https://github.com/apache/airflow/pull/11843#issuecomment-716185887
@mik-laj, could you 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #11843: Fix issue with moving end_date in elastic dag
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #11843:
URL: https://github.com/apache/airflow/pull/11843#discussion_r549502905
##########
File path: tests/test_utils/perf/dags/elastic_dag.py
##########
@@ -51,6 +51,24 @@ def parse_time_delta(time_str: str):
return timedelta(**time_params) # type: ignore
+def parse_start_date():
+ """
+ Returns the start date for the elastic DAGs and string to be used as part of their ids.
+
+ :return Tuple[datetime.datetime, str]: A tuple of datetime.datetime object to be used
+ as a start_date and a string that should be used as part of the dag_id.
+ """
+
+ if "PERF_START_DATE" in os.environ:
Review comment:
PERF_START_DATE env var and PERF_START_AGO env variable are mutually exclusive. Am I right? If so, it makes sense to me to throw an exception to warn the user of the unsupported configuration.
----------------------------------------------------------------
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] stale[bot] commented on pull request #11843: Fix issue with moving end_date in elastic dag
Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #11843:
URL: https://github.com/apache/airflow/pull/11843#issuecomment-751240004
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
----------------------------------------------------------------
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] olchas commented on a change in pull request #11843: Fix issue with moving end_date in elastic dag
Posted by GitBox <gi...@apache.org>.
olchas commented on a change in pull request #11843:
URL: https://github.com/apache/airflow/pull/11843#discussion_r554450379
##########
File path: tests/test_utils/perf/dags/elastic_dag.py
##########
@@ -51,6 +51,24 @@ def parse_time_delta(time_str: str):
return timedelta(**time_params) # type: ignore
+def parse_start_date():
+ """
+ Returns the start date for the elastic DAGs and string to be used as part of their ids.
+
+ :return Tuple[datetime.datetime, str]: A tuple of datetime.datetime object to be used
+ as a start_date and a string that should be used as part of the dag_id.
+ """
+
+ if "PERF_START_DATE" in os.environ:
Review comment:
Currently, if `PERF_START_DATE` is defined, then the value of `PERF_START_AGO` (if defined as well) will simply be ignored. Are you sure we should throw an exception if both of them are defined? They are mutually exclusive, but having both of them defined does not prevent parsing the dag file.
----------------------------------------------------------------
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