Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Dec 6, 2025

Shared secrets backend distribution should not import anything from airflow or airflow.sdk. So far it was detecting which connection class to return based on _AIRFLOW_PROCESS_CONTEXT but this was a bit brittle and required the shared class to know about the users.

This PR replaces it with injectong the connection class from outside by "set_connection_class". In both - task-sdk and airflow-core, the backends are initialized in a very similar way (this code will be soon extracted to common configuration) - but at the moment of initialization, we already know if we are in "airflow-core" or "task-sdk" context, so we can inject approproate Connection class.


^ 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 airflow-core/newsfragments.

Shared secrets backend distribution should not import anything
from airflow or airflow.sdk. So far it was detecting which connection
class to return based on _AIRFLOW_PROCESS_CONTEXT but this was a bit
brittle and required the shared class to know about the users.

This PR replaces it with injectong the connection class from
outside by "set_connection_class". In both - task-sdk and airflow-core,
the backends are initialized in a very similar way (this code will
be soon extracted to common configuration) - but at the moment of
initialization, we already know if we are in "airflow-core" or
"task-sdk" context, so we can inject approproate Connection class.
@potiuk
Copy link
Member Author

potiuk commented Dec 6, 2025

added as result of #58825

@potiuk
Copy link
Member Author

potiuk commented Dec 6, 2025

This one has a few caveats @amoghrajesh @ashb @kaxil @xBis7 :

  • because currently devel-common uses from airflow.models import configuration - shared tests are failing. We will only be able to get them succed when configuration is extracted to a shared distribution (and there we should inject connection class to configuration class as well rather than assume it is `from airflow.models'

  • this one is a little tricky - because SecretBackends are generally "public interface". The users who implement their own backend should implement it "from airflow.sdk.definitions" (and effectively from airlfow.sdk._shared)- but when the backend is used in "airflow-core" (i.e. api-server or scheduler) - the base class will be "from airflow._shared". This is not a big issue on it's own, but I think at some point in time we might implement a Protocol somewhere (currently we have no such place) which might be actually used by the users (rather than airflow.sdk.definitions) - because SecretsBackend is NOT only usable by "task-sdk" and yet we tell our users they should extend it.

Extending from "airflow.sdk.definitions" will still work, because effectively the Backends will implement the same Protocol and both base classes are implementing the same methods (because they have shared code). And we are already checking for at least attributes being as expected.

Not immediately, but something we have to think about for the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant