Skip to content

Conversation

@Taragolis
Copy link
Contributor

PR #25628 add new possibility with circular imports in Secrets Backends.
It affect both AWS secrets backends in case if user want to retrieve secrets configs from SB.

How to reproduce

export AIRFLOW__SECRETS__BACKEND=airflow.providers.amazon.aws.secrets.systems_manager.SystemsManagerParameterStoreBackend
❯ export AIRFLOW__SECRETS__BACKEND_KWARGS='{"config_prefix": "/airflow/config", "region_name": "eu-west-1"}'export AIRFLOW__DATABASE__SQL_ALCHEMY_CONN_SECRET=boom
❯ airflow config get-value database sql_alchemy_conn

...

airflow.exceptions.AirflowConfigException: Cannot retrieve config from alternative secrets backend. Make sure it is configured properly and that the Backend is accessible.
Cannot retrieve config from alternative secrets backend. Make sure it is configured properly and that the Backend is accessible.
cannot import name 'SessionFactory' from partially initialized module 'airflow.providers.amazon.aws.hooks.base_aws' (most likely due to a circular import) (/Users/taragolis/Projects/common/airflow/airflow/providers/amazon/aws/hooks/base_aws.py)

After this changes

export AIRFLOW__SECRETS__BACKEND=airflow.providers.amazon.aws.secrets.systems_manager.SystemsManagerParameterStoreBackend
❯ export AIRFLOW__SECRETS__BACKEND_KWARGS='{"config_prefix": "/airflow/config", "region_name": "eu-west-1"}'export AIRFLOW__DATABASE__SQL_ALCHEMY_CONN_SECRET=boom
❯ airflow config get-value database sql_alchemy_conn

[2022-09-29 22:27:59,204] {credentials.py:1180} INFO - Found credentials in environment variables.
[2022-09-29 22:27:59,665] {credentials.py:1180} INFO - Found credentials in environment variables.
sqlite:////Users/taragolis/airflow/airflow-from-ssm.db

closes: #26754

@boring-cyborg boring-cyborg bot added area:providers provider:amazon AWS/Amazon - related issues labels Sep 29, 2022
Copy link
Contributor

@vincbeck vincbeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

"""

conn: InitVar[Connection | AwsConnectionWrapper | None]
conn: InitVar[Connection | AwsConnectionWrapper | _ConnectionMetadata | None]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should consolidate these types a bit, perhaps with a Protocol.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree with you. Main problem with this class that when I initially create this helper I added InitVar and add some limitation with define attributes for this class. I have a plan to refactor it later and also get rid of _ConnectionMetadata.

@potiuk potiuk merged commit 8a1bbcf into apache:main Oct 6, 2022
@Taragolis Taragolis deleted the fix-circular-imports-amazon-sb branch January 14, 2023 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:amazon AWS/Amazon - related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot retrieve config from alternative secrets backend.

4 participants