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/06/13 05:37:59 UTC
[GitHub] [airflow] houqp opened a new pull request, #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
houqp opened a new pull request, #22699:
URL: https://github.com/apache/airflow/pull/22699
<!--
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/
-->
We have couple very critical DAGs that absolutely cannot be triggered manually ;)
---
**^ 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 diff in pull request #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #22699:
URL: https://github.com/apache/airflow/pull/22699#discussion_r845733347
##########
airflow/models/dag.py:
##########
@@ -2280,6 +2284,12 @@ def create_dagrun(
"Creating DagRun needs either `run_id` or both `run_type` and `execution_date`"
)
+ if not self.allow_manual and run_type == DagRunType.MANUAL:
+ raise AirflowException(
+ f"Cannot manually trigger DAG {self.dag_id}, because 'allow_manual' is "
+ f"set to {self.allow_manual}"
+ )
Review Comment:
Use a dedicated exception subclass here. You will need to catch this in a lot of places, and using `AirflowException` directly will make things very difficult.
--
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] houqp commented on pull request #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
houqp commented on PR #22699:
URL: https://github.com/apache/airflow/pull/22699#issuecomment-1092401944
@ashb @uranusjr if we pass the responsibility to `Timetable.infer_manual_data_interval`, how do we enforce this given users can override `infer_manual_data_interval`'s behavior with subclasses?
--
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] houqp commented on pull request #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
houqp commented on PR #22699:
URL: https://github.com/apache/airflow/pull/22699#issuecomment-1092411914
updated code to disable button in the UI
--
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] XLPI commented on pull request #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
XLPI commented on PR #22699:
URL: https://github.com/apache/airflow/pull/22699#issuecomment-1249963424
@houqp Are we going to follow up regarding this PR further?
--
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 #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22699:
URL: https://github.com/apache/airflow/pull/22699#issuecomment-1201862933
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] houqp commented on a diff in pull request #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
houqp commented on code in PR #22699:
URL: https://github.com/apache/airflow/pull/22699#discussion_r842193253
##########
airflow/models/dag.py:
##########
@@ -2347,6 +2352,19 @@ def create_dagrun(
creating_job_id=creating_job_id,
data_interval=data_interval,
)
+
+ # prevent triggering manually for scheduled DAGs, while 'allow_manual' = False
+ if (
+ run.run_type == DagRunType.MANUAL
+ and self.schedule_interval is not None
+ and self.schedule_interval != "@once"
Review Comment:
good call @uranusjr , updated. FYI @XLPI .
--
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] closed pull request #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
URL: https://github.com/apache/airflow/pull/22699
--
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 #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #22699:
URL: https://github.com/apache/airflow/pull/22699#issuecomment-1091970513
That’d work. Another solution I can think of is to introduce a helper class:
```python
from airflow.timetables import ScheduleInterval
DAG(timetable=ScheduleInterval(what_normally_goes_to_schedule_interval, allow_manual=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 pull request #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #22699:
URL: https://github.com/apache/airflow/pull/22699#issuecomment-1091911716
Adding it to the timetable sounds like the right thing, but has the obvious downside of making easy cases currently using `schedule_interval` much more verbose.
--
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] closed pull request #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
URL: https://github.com/apache/airflow/pull/22699
--
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 diff in pull request #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #22699:
URL: https://github.com/apache/airflow/pull/22699#discussion_r847768433
##########
airflow/exceptions.py:
##########
@@ -298,3 +298,7 @@ def __repr__(self) -> str:
class TaskDeferralError(AirflowException):
"""Raised when a task failed during deferral for some reason."""
+
+
+class DAGDisallowManual(AirflowException):
Review Comment:
```suggestion
class ManualDAGRunDisallowed(AirflowException):
```
seems to read better forme.
--
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] bbovenzi commented on pull request #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on PR #22699:
URL: https://github.com/apache/airflow/pull/22699#issuecomment-1091799674
Yes, I agree that we should also disable the button. We do this a bunch in the html files already. [Example in dag.html](https://github.com/apache/airflow/blob/main/airflow/www/templates/airflow/dag.html#L109)
--
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 #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22699:
URL: https://github.com/apache/airflow/pull/22699#issuecomment-1249950507
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.
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 #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22699:
URL: https://github.com/apache/airflow/pull/22699#issuecomment-1301513098
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.
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 #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #22699:
URL: https://github.com/apache/airflow/pull/22699#issuecomment-1095613028
You need to find all `create_dagrun` calls that creates manual runs, and decide how this needs to be catched and handled. I believe there are at least two, one in CLI and one in web UI, and you need to emit the correct response in those situations as appropriate.
--
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 #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22699:
URL: https://github.com/apache/airflow/pull/22699#issuecomment-1149299364
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.
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 #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
ashb commented on PR #22699:
URL: https://github.com/apache/airflow/pull/22699#issuecomment-1088542458
Rather than adding another argument to DAG constructor does it maybe make more sense to add this as an argument to the Timetable?
```python
with DAG(dag_id="dag_id", timetable=CronDataIntervalTimetable("0 0 * * *", allow_manual=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] houqp commented on a diff in pull request #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
houqp commented on code in PR #22699:
URL: https://github.com/apache/airflow/pull/22699#discussion_r842193390
##########
airflow/models/dag.py:
##########
@@ -2347,6 +2352,19 @@ def create_dagrun(
creating_job_id=creating_job_id,
data_interval=data_interval,
)
+
+ # prevent triggering manually for scheduled DAGs, while 'allow_manual' = False
+ if (
+ run.run_type == DagRunType.MANUAL
+ and self.schedule_interval is not None
+ and self.schedule_interval != "@once"
+ and not self.allow_manual
+ ):
+ raise AirflowException(
+ f"Cannot manually trigger DAG {self.dag_id}, because it is a scheduled DAG, "
+ f"and 'allow_manual' is set to {self.allow_manual}"
Review Comment:
I have added a test case 👍
--
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 #22699: add allow_manual DAG argument to prevent scheduled DAGs from being triggered manually
Posted by GitBox <gi...@apache.org>.
ashb commented on PR #22699:
URL: https://github.com/apache/airflow/pull/22699#issuecomment-1091920130
Oh yeah -- maybe we move the handling of the param to Timetable classes, but accept the parameter on the DAG constructor?
--
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