Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a way to import Airflow without side-effects #25832

Merged
merged 2 commits into from
Aug 19, 2022

Conversation

andrewgodwin
Copy link
Contributor

I know it's been a long-standing issue that it should be possible to import Airflow as a library without side-effects, and while I think the ultimate fix for this is to go through and steadily remove the need to call settings.initalize() in __init__.py, I am currently working on a project where I would really like to use Airflow without it doing strange things to logging, sys.path, or atexit.

As such, this PR wraps the import side-effect in a simple environment variable check that code can use to disable the side-effects for now, while we slowly try and progress towards a "cleaner" solution. Without this, I am having to dynamically modify the source code of __init__.py in an import hook, and nobody wants that!

I don't believe it's possible to write tests for this as Airflow is already imported when tests are running, but if you can think of a way, let me know and I'll have a go. The environment variable name is also up for consideration - I just picked something that looked a bit like a setting, but we can deliberately make it not look like that if we want.

Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

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

As tactical fixes go, sure. Probably worth adding a comment about "warning, doing this will probably have odd side effects" if someone goes spelunking.

I think it's also favour a _ prefix on the env var to make it "private"

@andrewgodwin
Copy link
Contributor Author

I like both those ideas - I have added a warning and the private prefix.

@kaxil kaxil merged commit 5e5cf63 into apache:main Aug 19, 2022
@andrewgodwin andrewgodwin deleted the aslibrary branch August 19, 2022 17:24
@potiuk
Copy link
Member

potiuk commented Aug 19, 2022

Yeah. good tactical solution :). But eventually I think we should lazy intialize everything on first use (but that's far more complex change).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:new-feature Changelog: New Features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants