potiuk commented on code in PR #25810:
URL: https://github.com/apache/airflow/pull/25810#discussion_r950265535
##########
airflow/providers/amazon/aws/secrets/secrets_manager.py:
##########
@@ -193,8 +196,11 @@ def _format_uri_with_extra(secret, conn_string: str) ->
str:
return conn_string
- def get_connection(self, conn_id: str) -> Optional[Connection]:
+ def get_connection(self, conn_id: str) -> Optional["Connection"]:
if not self.full_url_mode:
+ # Avoid circular import problems when instantiating the backend
during configuration.
+ from airflow.models.connection import Connection
Review Comment:
Yeah. We definitely import too much in some core packages.
And we should indeed add secrets manager tests. But I think #24486 is not
the solution to this problem. It is a good approach but we have a problem with
internal architecture, not with imports.
The real problem is not importing stuff, the problem is that we actually
HAVE a circular dependency here and it's because our configuration approach is
pretty badly intertwined with other parts of the system in this case we have:
* configuration can read values by delegating to secrets manager
* secrets manager uses configuration to configure itself
While I agree local imports and TYPE_CHECKING can provide some solution to
the problem, they are very prone to trigger similar problems.
I think we should finally do something about it - I think ti's a bad
approach that two independent parts of systems are depending on each other :).
The dependency between those should be one way - either conf is using secrets
backend or secrets backend using conf. Basically we have `secrets -> conf ->
secret` dependency chain which makes it very easy to turn it into circular
import problem.
Probably the best solution is to split out the part that secret REALLY need
from conf and make it an "internal conf" (and then `conf -> secret -> "internal
conf"` will be the correct dependency chain that will not lead to accidental
triggering of such cricular imports.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
