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 2022/03/31 19:29:36 UTC
[GitHub] [airflow] frankbreetz opened a new pull request #22666: Cli next execution bug
frankbreetz opened a new pull request #22666:
URL: https://github.com/apache/airflow/pull/22666
<!--
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/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/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/main/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.
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 #22666: Cli next execution bug
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #22666:
URL: https://github.com/apache/airflow/pull/22666#discussion_r840232050
##########
File path: airflow/cli/cli_parser.py
##########
@@ -273,6 +273,11 @@ def string_lower_type(val):
type=positive_int(allow_zero=False),
help="The number of next execution datetimes to show",
)
+ARG_RESPECT_CATCHUP = Arg(
+ ("-r", "--respect-catchup"),
+ action="store_true",
+ help="Respect catchup=False on a paused DAG",
Review comment:
```suggestion
help="Respect the DAG's catchup 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.
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 #22666: Cli next execution bug
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #22666:
URL: https://github.com/apache/airflow/pull/22666#discussion_r840262850
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -276,6 +277,17 @@ def dag_next_execution(args):
if dag.get_is_paused():
print("[INFO] Please be reminded this DAG is PAUSED now.", file=sys.stderr)
+ if args.respect_catchup:
+ dag_run = DagRun(execution_date=datetime.utcnow())
+ next_info = dag.next_dagrun_info(dag.get_run_data_interval(dag_run), restricted=False)
+ print(next_info.logical_date.isoformat())
+ return
+ elif not dag.catchup and not args.respect_catchup:
+ print(
+ "[INFO] Please be reminded this DAG has catchup=False."
+ "if you would like to base command off current time pass -r flag",
+ file=sys.stderr,
+ )
Review comment:
I think instead of consistently telling the user about this, we should gradually deprecate the current behaviour, trasition most people to the new behaviour, but still allow the user to explicitly specify the current behaviour without being further disturbed.
So I think we should add two [mutually exclusive options](https://docs.python.org/3/library/argparse.html#mutual-exclusion): `--respect-catchup` and `--no-respect-catchup`, and
1. If `--respect-catchup` is specified, use the new behaviour.
2. If `--no-repsect-catchup` is specified, use the current behaviour.
3. If neither is specified, use the current behaviour, _and emit a deprecation warning_ describing this does not respect the catchup setting, the behaviour will change in Airflow 3.0, and the user is advised to explicitly pass either argument to suppress the warning.
By the way, the `-r` shorthand does not seem right to me because the main thing about the options is _catchup_, not _respect_; I’d say let’s **not** add a shorthand for now, we can always do that later.
--
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 #22666: Cli next execution bug
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #22666:
URL: https://github.com/apache/airflow/pull/22666#discussion_r840231314
##########
File path: .gitignore
##########
@@ -77,7 +77,7 @@ coverage*.xml
local_settings.py
# Flask stuff:
-instance/
+instance/:
Review comment:
Why this change?
--
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