potiuk commented on pull request #16088: URL: https://github.com/apache/airflow/pull/16088#issuecomment-849323125
> Looks good to me. I wonder if it would be possible to add a test for this; could be difficult since it depends on the global interpreter state? Yeah. Testing this is difficult. And actually we kind'a test it in the updated test -> the sequence `initialize` followed by `validate` is run there (that's why I actually added the "validate()" in the test even if it was not strictly necessary there. But you are right. By the time we got there in tests the "initialize()" command have been run multiple times already in other tests and global state is already changed. I think it would require some refactoring of the approach we use conf. Currently it is indeed a global state, but it should really be independent object that you can initialize and test in isolation. This is what I mentioned in the other related comment in https://github.com/apache/airflow/issues/15685#issuecomment-834181697 . The way `configuration` is used is the main reason why we cannot remove the last few files from `pylint_todo.txt` - because pylint (rightfully) detects cyclic dependencies (not imports!) because of the way configuration is doing two things - it is both `used by` and `uses` other airflow components. For now this problem seems to be fixed by this change (confirmed by @dowthron in #16079 - seems @downthron came to the same fix as I did in the meantime). But I am happy to discuss the way we can approach configuration differently as next step. The little problem with that is that it might only be really fixable in Airflow 3.0 because it might introduce some backward-incompatibilities as we might need to change how `airflow.configuration.conf` object is used and I believe people use it as a "public API" of airflow and by changing it we might break their plugins and dags. But maybe we can do it similarly as we did with lazy initialization of conf rather than import via __getattr__ /PEP-562 + STATICA_HACK https://github.com/apache/airflow/blob/71ef2f2ee9ccf238a99cb0e42412d2118bad22a1/airflow/__init__.py#L55 to handle 'from airflow import DAG` and `from airflow import AirflowException`. It would be great to have separate conf object that we could test initialization of independently and untangle the cyclic dependencies. -- 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. For queries about this service, please contact Infrastructure at: [email protected]
