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

Mark the hardcoded hybrid executors as deprecate and multi-exec as stable #46944

Open
wants to merge 10 commits into
base: v2-10-stable
Choose a base branch
from

Conversation

o-nikolas
Copy link
Contributor

@o-nikolas o-nikolas commented Feb 20, 2025

In preparation of deprecating support in Airflow core for the old hardcoded hybrid executors in V3, mark them as deprecated and emit warnings when they are loaded. Also mark multi exec as stable to give users somewhere to migrate towards.

To broadcast the message of their deprecation this PR:
- Adds a deprecation warning when these old hybrid executors are loaded
- Documentation has been updated to note that they are deprecated and provide steps for migration.
- A news fragment was added as well.

The warning message:
Screenshot from 2025-02-20 11-57-47

Rendered docs:
Screenshot from 2025-02-20 13-54-20


^ 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.

Multiple Executor Configuration (aka hybrid executors) has been released
for over half a year and can be marked stable. This gives users an
option to migrate to from the old hardcoded hybrid executors (e.g
CeleryKubernetesExecutor)
These executors will be removed in Airflow 3.0. Mark them as deprecated
so users are aware and have time to migrate.

- A deprecation warning will be printed when the executors are loaded
- Documentation has been updated to note that they are deprecated
- A news fragment added as well.
@o-nikolas
Copy link
Contributor Author

CC: @jedcunningham @jscheffl @amoghrajesh

PR to mark the static executors as deprecated in Airflow core and mark multi-exec as stable. Re: https://lists.apache.org/thread/qmdf5dx71lzcvf4fd7qyc9gq0z0j2jqr

Copy link
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

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

Looks good in my view

@o-nikolas
Copy link
Contributor Author

Getting the tests to ignore the deprecation warning at the right time has proven very annoying/tricky.

The last stubborn one is related to one instances of the warning not being thrown, but it seems that some underlying failure is actually happening, but I'm not sure why it's happening on this branch that should be otherwise stable. And I also cannot produce it locally. From the test failure logs:

                with pytest.warns(RemovedInAirflow3Warning):
>                   file_handler.read(ti)
E                   Failed: DID NOT WARN. No warnings of type (<class 'airflow.exceptions.RemovedInAirflow3Warning'>,) were emitted.
E                    Emitted warnings: [].

tests/utils/test_log_handlers.py:289: Failed
---------------------------- Captured stdout setup -----------------------------
[2025-02-21T01:46:21.370+0000] {dagbag.py:588} INFO - Filling up the DagBag from /dev/null
----------------------------- Captured stdout call -----------------------------
[2025-02-21T01:46:21.386+0000] {dag.py:3239} INFO - Sync 1 DAGs
[2025-02-21T01:46:21.391+0000] {dag.py:3262} INFO - Creating ORM DAG for dag_for_testing_multiple_executors
[2025-02-21T01:46:21.397+0000] {dag.py:4180} INFO - Setting next_dagrun for dag_for_testing_multiple_executors to 2016-01-01T00:00:00+00:00, run_after=2016-01-02T00:00:00+00:00
[2025-02-21T01:46:21.436+0000] {file_task_handler.py:623} ERROR - Could not read served logs
Traceback (most recent call last):
  File "/opt/airflow/airflow/utils/log/file_task_handler.py", line 599, in _read_from_logs_server
    log_type = LogType.TRIGGER if ti.triggerer_job else LogType.WORKER
  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/ext/associationproxy.py", line 193, in __get__
    return inst.get(obj)
  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/ext/associationproxy.py", line 575, in get
    target = getattr(obj, self.target_collection)
  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/orm/attributes.py", line 487, in __get__
    return self.impl.get(state, dict_)
  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/orm/attributes.py", line 959, in get
    value = self._fire_loader_callables(state, key, passive)
  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/orm/attributes.py", line 995, in _fire_loader_callables
    return self.callable_(state, passive)
  File "/usr/local/lib/python3.9/site-packages/sqlalchemy/orm/strategies.py", line 863, in _load_for_state
    raise orm_exc.DetachedInstanceError(
sqlalchemy.orm.exc.DetachedInstanceError: Parent instance <TaskInstance at 0x7fe810c4d6d0> is not bound to a Session; lazy load operation of attribute 'trigger' cannot proceed (Background on this error at: https://sqlalche.me/e/14/bhk3)

Copy link
Contributor

@amoghrajesh amoghrajesh left a comment

Choose a reason for hiding this comment

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

LGTM

We already have coverage for this behaviour with the other executor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Executors-core LocalExecutor & SequentialExecutor kind:documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants