You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "uranusjr (via GitHub)" <gi...@apache.org> on 2025/07/30 09:39:29 UTC
[PR] Remove read_dags_from_db=True usages [airflow]
uranusjr opened a new pull request, #53918:
URL: https://github.com/apache/airflow/pull/53918
* [ ] Remove DagBag usages where `read_dags_from_db=True`
* [ ] Remove db access from DagBag
* [ ] Move DagBag to SDK (for use in dag parser and task runner)
--
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
Re: [PR] Move DagBag to SDK and make it return SDK DAG objects [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #53918:
URL: https://github.com/apache/airflow/pull/53918#issuecomment-3176996294
Initially I planned to remove all db access, and move the DagBag class to SDK. However, after spending almost a week trying to debug some weird Kubernetes integration test failures, I give up. This no longer moves the class.
I removed all database access in DagBag _except `get_dag`, where it checks whether a dag has expired and auto-refreshes it_. I think we no longer needs this check since dagbag uses are ephemeral in Airflow now, but we will need to fix test cases to reflect that. This will be done in a later PR.
--
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
Re: [PR] Move DagBag to SDK and make it return SDK DAG objects [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #53918:
URL: https://github.com/apache/airflow/pull/53918#discussion_r2275953231
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/ui/test_dashboard.py:
##########
@@ -34,9 +34,7 @@
@pytest.fixture(autouse=True, scope="module")
def examples_dag_bag():
- # Speed up: We don't want example dags for this module
-
- return DagBag(include_examples=False, read_dags_from_db=True)
+ return DBDagBag()
Review Comment:
Looks like we rely on some cleanups in each test module. Tests are to fail when I move this. I’m going to roll back the change; this doesn’t warrant too much thought IMO.
--
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
Re: [PR] Move DagBag to SDK and make it return SDK DAG objects [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #53918:
URL: https://github.com/apache/airflow/pull/53918#discussion_r2275704931
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/ui/test_dashboard.py:
##########
@@ -34,9 +34,7 @@
@pytest.fixture(autouse=True, scope="module")
def examples_dag_bag():
- # Speed up: We don't want example dags for this module
-
- return DagBag(include_examples=False, read_dags_from_db=True)
+ return DBDagBag()
Review Comment:
I moved it to `airflow-core/tests/unit/api_fastapi/core_api/routes/ui/conftest.py` for now since it’s not used outside the directory. I also made it session-scoped since it does not need to be created (there’s no state at all).
We can always change this in the future. Moving it to `devel-common` is likely not necessary since DBDagBag does not make sense outside `airflow-core`, but we can move it to a broader conftest file if needed.
--
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
Re: [PR] Remove read_dags_from_db=True usages [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #53918:
URL: https://github.com/apache/airflow/pull/53918#issuecomment-3135553264
The Databricks operator links are a mess… Not dealing with them right now, I believe they are already broken in 3.0 so we can just keep breaking it more.
--
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
Re: [PR] Move DagBag to SDK and make it return SDK DAG objects [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr merged PR #53918:
URL: https://github.com/apache/airflow/pull/53918
--
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
Re: [PR] Move DagBag to SDK and make it return SDK DAG objects [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #53918:
URL: https://github.com/apache/airflow/pull/53918#discussion_r2269622500
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/ui/test_dashboard.py:
##########
@@ -34,9 +34,7 @@
@pytest.fixture(autouse=True, scope="module")
def examples_dag_bag():
- # Speed up: We don't want example dags for this module
-
- return DagBag(include_examples=False, read_dags_from_db=True)
+ return DBDagBag()
Review Comment:
How about moving `examples_dag_bag` fixture to `devel-common` ?
Since `examples_dag_bag` fixture is required for the following 3 UI route tests.
--
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
Re: [PR] Move DagBag to SDK and make it return SDK DAG objects [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #53918:
URL: https://github.com/apache/airflow/pull/53918#issuecomment-3182384884
I think we ultimately should remove the functionality. Scheduler and API server (the long running processes) already do not use this class now. The worker and dag parsing process are short-lived and this refreshing logic should not kick in at all. The main issue is to fix the tests so they reflect this, and implement necessary refreshing when a test expect it to happen automatically in-process (but now only happens in the dag processor in production circumstances).
--
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