Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Run task action from UI #29706

Merged
merged 4 commits into from
Feb 23, 2023
Merged

Conversation

bbovenzi
Copy link
Contributor

@bbovenzi bbovenzi commented Feb 22, 2023

Airflow.run is broken. It tries to start an executor. From the UI, users should use Clear instead. This PR removes the Run buttons from the UI (we will only show clear or mark failure/success).

Note: Since it is broken, I feel like this counts as a bug fix for 2.5.2. But if removing a UI element is too drastic I'm fine to move this to 2.6.0 instead

Before:
Screenshot 2023-02-22 at 4 07 09 PM

After:
Screenshot 2023-02-22 at 4 02 08 PM


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@bbovenzi bbovenzi added this to the Airflow 2.5.2 milestone Feb 22, 2023
@boring-cyborg boring-cyborg bot added area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Feb 22, 2023
@bbovenzi bbovenzi requested a review from kaxil February 22, 2023 21:04
Copy link
Member

@kaxil kaxil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to think if we should remove it from

airflow/airflow/www/views.py

Lines 1848 to 1904 in 8449180

def run(self, session=None):
"""Runs Task Instance."""
dag_id = request.form.get("dag_id")
task_id = request.form.get("task_id")
dag_run_id = request.form.get("dag_run_id")
map_index = request.args.get("map_index", -1, type=int)
origin = get_safe_url(request.form.get("origin"))
dag = get_airflow_app().dag_bag.get_dag(dag_id)
if not dag:
return redirect_or_json(origin, "DAG not found", "error", 404)
task = dag.get_task(task_id)
ignore_all_deps = request.form.get("ignore_all_deps") == "true"
ignore_task_deps = request.form.get("ignore_task_deps") == "true"
ignore_ti_state = request.form.get("ignore_ti_state") == "true"
executor = ExecutorLoader.get_default_executor()
if not executor.supports_ad_hoc_ti_run:
msg = f"{executor.__class__.__name__} does not support ad hoc task runs"
return redirect_or_json(origin, msg, "error", 400)
dag_run = dag.get_dagrun(run_id=dag_run_id, session=session)
if not dag_run:
return redirect_or_json(origin, "DAG run not found", "error", 404)
ti = dag_run.get_task_instance(task_id=task.task_id, map_index=map_index, session=session)
if not ti:
msg = "Could not queue task instance for execution, task instance is missing"
return redirect_or_json(origin, msg, "error", 400)
ti.refresh_from_task(task)
# Make sure the task instance can be run
dep_context = DepContext(
deps=RUNNING_DEPS,
ignore_all_deps=ignore_all_deps,
ignore_task_deps=ignore_task_deps,
ignore_ti_state=ignore_ti_state,
)
failed_deps = list(ti.get_failed_dep_statuses(dep_context=dep_context))
if failed_deps:
failed_deps_str = ", ".join(f"{dep.dep_name}: {dep.reason}" for dep in failed_deps)
msg = f"Could not queue task instance for execution, dependencies not met: {failed_deps_str}"
return redirect_or_json(origin, msg, "error", 400)
executor.job_id = None
executor.start()
executor.queue_task_instance(
ti,
ignore_all_deps=ignore_all_deps,
ignore_task_deps=ignore_task_deps,
ignore_ti_state=ignore_ti_state,
)
executor.heartbeat()
ti.queued_dttm = timezone.utcnow()
session.merge(ti)
msg = f"Sent {ti} to the message queue, it should start any moment now."
return redirect_or_json(origin, msg)

🤔

@dimberman
Copy link
Contributor

@kaxil what would be the benefit of keeping it?

@eladkal
Copy link
Contributor

eladkal commented Feb 23, 2023

the run command is also available from CLI. I'm missing the part why it makes sense to use it in CLI but not from the UI? the end result is the same doesn't it?

@jedcunningham
Copy link
Member

The CLI will be a little trickier, but yes, the non --local or --raw path which spins up an executor should probably also be axed.

@jedcunningham
Copy link
Member

And --local certainly makes sense - it's how KubernetesExecutor works. I'm not familiar enough with --raw to speak to that one though.

@uranusjr
Copy link
Member

IIRC --raw is needed for (some?) other executors to run the task.

@bbovenzi bbovenzi modified the milestones: Airflow 2.5.2, Airflow 2.6.0 Feb 23, 2023
@jedcunningham
Copy link
Member

Yeah, when troubleshooting something else today, I happened to find that --local ends up using --raw as well 👍. Either way, I think we should address the UI and CLI separately.

@bbovenzi bbovenzi merged commit 101d59c into apache:main Feb 23, 2023
@bbovenzi bbovenzi deleted the remove-run-task-action branch February 23, 2023 22:22
@ephraimbuddy ephraimbuddy added type:improvement Changelog: Improvements type:bug-fix Changelog: Bug Fixes and removed type:improvement Changelog: Improvements labels Apr 11, 2023
o-nikolas added a commit to aws-mwaa/upstream-to-airflow that referenced this pull request Aug 14, 2023
The run task instance capability was removed from the Airflow UI and API
in apache#29706 but the base executor interface and subclasses which support
it was not updated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants