You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "o-nikolas (via GitHub)" <gi...@apache.org> on 2023/02/14 01:09:37 UTC
[GitHub] [airflow] o-nikolas opened a new pull request, #29523: Fix import cycle starting from airflow/__main__.py
o-nikolas opened a new pull request, #29523:
URL: https://github.com/apache/airflow/pull/29523
This is a fix related to #29494, this fix stops the cycle at the source by importing configuration first before anything else so that the `conf` object can be built and validated outside the scope of `airflow.settings`.
## Please see see #29257 for more details, as it has a fuller explanation for how this fix works
<!--
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 an 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 changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+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 a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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] jedcunningham commented on a diff in pull request #29523: Fix import cycle starting from airflow/__main__.py
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #29523:
URL: https://github.com/apache/airflow/pull/29523#discussion_r1106179817
##########
airflow/__main__.py:
##########
@@ -24,12 +24,21 @@
import argcomplete
+# The configuration module initializes and validates the conf object as a side effect the first
+# time it is imported. If it is not imported before importing the settings module, the conf
+# object will then be initted/validated as a side effect of it being imported in settings,
+# however this can cause issues since those modules are very tightly coupled and can
+# very easily cause import cycles in the conf init/validate code (since downstream code from
+# those functions likely import settings).
+# Therefore importing configuration early (as the first airflow import) avoids
+# any possible import cycles with settings downstream.
+from airflow import configuration
Review Comment:
Why not import `conf` directly here?
--
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] o-nikolas commented on pull request #29523: Fix import cycle starting from airflow/__main__.py
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on PR #29523:
URL: https://github.com/apache/airflow/pull/29523#issuecomment-1430149593
Cc-ing @potiuk and @jedcunningham who reviewed the same fix for `__init__.py`
--
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] o-nikolas commented on a diff in pull request #29523: Fix import cycle starting from airflow/__main__.py
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on code in PR #29523:
URL: https://github.com/apache/airflow/pull/29523#discussion_r1106325155
##########
airflow/__main__.py:
##########
@@ -24,12 +24,21 @@
import argcomplete
+# The configuration module initializes and validates the conf object as a side effect the first
+# time it is imported. If it is not imported before importing the settings module, the conf
+# object will then be initted/validated as a side effect of it being imported in settings,
+# however this can cause issues since those modules are very tightly coupled and can
+# very easily cause import cycles in the conf init/validate code (since downstream code from
+# those functions likely import settings).
+# Therefore importing configuration early (as the first airflow import) avoids
+# any possible import cycles with settings downstream.
+from airflow import configuration
Review Comment:
It was making ruff upset doing it that way. Using `from airflow import ...` allows it to be the first import in the list without linters being angry.
--
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] jedcunningham merged pull request #29523: Fix import cycle starting from airflow/__main__.py
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham merged PR #29523:
URL: https://github.com/apache/airflow/pull/29523
--
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