Remove Connection dependency from shared secrets backend#61523
Merged
amoghrajesh merged 14 commits intoapache:mainfrom Mar 2, 2026
Merged
Remove Connection dependency from shared secrets backend#61523amoghrajesh merged 14 commits intoapache:mainfrom
amoghrajesh merged 14 commits intoapache:mainfrom
Conversation
Contributor
Author
|
@potiuk I vaguely remembering being tagged in one such task from you, or maybe I am mistaken but hopefully this was the one! |
Contributor
Author
|
Most of the failures in here are handled by: #61728 |
o-nikolas
reviewed
Feb 23, 2026
Contributor
o-nikolas
left a comment
There was a problem hiding this comment.
The tests are still failing, but the code looks reasonable to me. Though, I don't work much on the SDK/Core boundary so take this review with a large grain of salt 😅
Contributor
Author
|
Thanks for the review @o-nikolas! It is dependent on #62211 |
1 task
Contributor
Author
7359e45 to
24bb6af
Compare
uranusjr
approved these changes
Mar 2, 2026
Contributor
Author
|
Finally! Merging this one |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Was generative AI tooling used to co-author this PR?
Why?
The shared
BaseSecretsBackendwas importing Connection classes from airflow-core or airflow-sdk conditionally based on the current process context, creating a dependency that breaks client-server separation in ways. This change removes that dependency by injecting the Connection class per instance at distribution init time.Both core and sdk have backend loaders:
initialize_secrets_backendswhich now set the Connection class oneach backend instance via
_set_connection_class()when instantiating their backends. This ensures each backend uses the correct Connection type (core or sdk) based on where it's created, without the shared base needing to know about either.Not so ideal, but there could be cases when the secrets backend class is instantiated manually and used, to account for such cases, we have a fallback which sets
Connectionfrom respective library / consuming module on its re export of BaseSecretsBackend. ie:_connection_classis stored on the instance (self._connection_class).The “default” is just the logic that decides what to set when the instance has nothing; it’s not a shared class attribute.
The shared base now has zero imports of core or SDK Connection types, maintaining proper separation between components.
Design Decision and Alternatives
Why per instance instead of class level default?
I considered a class level
_default_connection_classattribute, but didn't choose it because:Per instance storage avoids all these issues by making each backend independent.
Why not use a factory/callable?
I also considered storing a callable that returns the Connection class, but chose direct class storage over that one cos:
Back compat
initialize_secrets_backends. So, they automatically get the correct Connection class set on the instances.{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.