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

Adding UIAlert in local settings fails with Sentry enabled #31442

Open
1 of 2 tasks
potiuk opened this issue May 21, 2023 Discussed in #31408 · 7 comments
Open
1 of 2 tasks

Adding UIAlert in local settings fails with Sentry enabled #31442

potiuk opened this issue May 21, 2023 Discussed in #31408 · 7 comments
Labels
affected_version:2.6 Issues Reported for 2.6 area:core kind:bug This is a clearly a bug

Comments

@potiuk
Copy link
Member

potiuk commented May 21, 2023

Discussed in #31408

Originally posted by gil-tober May 18, 2023

Apache Airflow version

2.6.1

What happened

When upgrading Airflow from 2.5.3 to 2.6.1 the airflow-run-migration job fails.

I have deployed the same setup to our DEV environment (where Sentry is disabled) and the upgrade succeeded without an issue.

After that I tried deploying to our PROD environment where Sentry is enabled and the job failed with the error below.

I tried enabling Sentry on DEV and the upgrade failed.

Took a look at airflow.executors.executor_loader.py at lines 162-185 when the exception is raised from. Setting the environment variable _AIRFLOW__SKIP_DATABASE_EXECUTOR_COMPATIBILITY_CHECK=1 fixed the error getting in the upgrade job. (Our metadb is Postgres)

Traceback (most recent call last):
  File “/home/airflow/.local/bin/airflow”, line 5, in <module>
    from airflow.__main__ import main
  File “/home/airflow/.local/lib/python3.10/site-packages/airflow/__init__.py”, line 66, in <module>
    settings.initialize()
  File “/home/airflow/.local/lib/python3.10/site-packages/airflow/settings.py”, line 522, in initialize
    import_local_settings()
  File “/home/airflow/.local/lib/python3.10/site-packages/airflow/settings.py”, line 467, in import_local_settings
    import airflow_local_settings
  File “/opt/airflow/dags/repo/airflow_local_settings.py”, line 5, in <module>
    from airflow.www.utils import UIAlert
  File “/home/airflow/.local/lib/python3.10/site-packages/airflow/www/utils.py”, line 44, in <module>
    from airflow.models.dagrun import DagRun
  File “/home/airflow/.local/lib/python3.10/site-packages/airflow/models/dagrun.py”, line 57, in <module>
    from airflow.models.taskinstance import TaskInstance as TI
  File “/home/airflow/.local/lib/python3.10/site-packages/airflow/models/taskinstance.py”, line 100, in <module>
    from airflow.sentry import Sentry
  File “/home/airflow/.local/lib/python3.10/site-packages/airflow/sentry.py”, line 195, in <module>
    Sentry = ConfiguredSentry()
  File “/home/airflow/.local/lib/python3.10/site-packages/airflow/sentry.py”, line 92, in __init__
    executor_class, _ = ExecutorLoader.import_default_executor_cls()
  File “/home/airflow/.local/lib/python3.10/site-packages/airflow/executors/executor_loader.py”, line 158, in import_default_executor_cls
    executor, source = cls.import_executor_cls(executor_name)
  File “/home/airflow/.local/lib/python3.10/site-packages/airflow/executors/executor_loader.py”, line 134, in import_executor_cls
    return _import_and_validate(cls.executors[executor_name]), ConnectorSource.CORE
  File “/home/airflow/.local/lib/python3.10/site-packages/airflow/executors/executor_loader.py”, line 130, in _import_and_validate
    cls.validate_database_executor_compatibility(executor)
  File “/home/airflow/.local/lib/python3.10/site-packages/airflow/executors/executor_loader.py”, line 181, in validate_database_executor_compatibility
    from airflow.settings import engine
ImportError: cannot import name ‘engine’ from ‘airflow.settings’ (/home/airflow/.local/lib/python3.10/site-packages/airflow/settings.py)

What you think should happen instead

migrateDatabaseJob should not fail when Sentry is enabled

How to reproduce

Upgrade from 2.5.3 to 2.6.1 with CeleryKubernetesExecutor and Sentry enabled

Operating System

Debian GNU/Linux 11 (bullseye)

Versions of Apache Airflow Providers

apache-airflow-providers-amazon==8.0.0
apache-airflow-providers-celery==3.1.0
apache-airflow-providers-cncf-kubernetes==6.1.0
apache-airflow-providers-common-sql==1.4.0
apache-airflow-providers-docker==3.6.0
apache-airflow-providers-elasticsearch==4.4.0
apache-airflow-providers-ftp==3.3.1
apache-airflow-providers-google==8.3.0
apache-airflow-providers-grpc==3.1.0
apache-airflow-providers-hashicorp==3.3.1
apache-airflow-providers-http==4.3.0
apache-airflow-providers-imap==3.1.1
apache-airflow-providers-jenkins==3.2.1
apache-airflow-providers-microsoft-azure==6.0.0
apache-airflow-providers-mysql==5.0.0
apache-airflow-providers-odbc==3.2.1
apache-airflow-providers-postgres==5.3.1
apache-airflow-providers-redis==3.1.0
apache-airflow-providers-salesforce==5.3.0
apache-airflow-providers-sendgrid==3.1.0
apache-airflow-providers-sftp==4.2.4
apache-airflow-providers-slack==7.2.0
apache-airflow-providers-snowflake==4.0.5
apache-airflow-providers-sqlite==3.3.2
apache-airflow-providers-ssh==3.6.0
apache-airflow-providers-tableau==4.1.0

Deployment

Official Apache Airflow Helm Chart

Deployment details

Airflow deployed on an EKS cluster using the official Airflow Helm Chart (V1.9.0)
MetaBD is a Postgres RDS
CeleryKubernetesExecutor

Anything else

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@potiuk potiuk added kind:bug This is a clearly a bug pending-response area:core affected_version:2.6 Issues Reported for 2.6 labels May 21, 2023
@potiuk
Copy link
Member Author

potiuk commented May 21, 2023

comment: Seems that indeed we have a circular import happening when UIAlert is created in local_settins.

@potiuk potiuk added this to the Airflow 2.6.2 milestone May 21, 2023
@potiuk
Copy link
Member Author

potiuk commented May 21, 2023

Very interesting bug :) .

The root cause is AIP-51 related change #28375 which added executor initialization in Sentry's ConfiguredSentry() __init__ class. This is done only in order to see if the executor supports sentry.

            if executor_class.supports_sentry:
                from sentry_sdk.integrations.celery import CeleryIntegration

                sentry_celery = CeleryIntegration()
                integrations.append(sentry_celery)

It's an interesting sequence of events:

  • local settings are being imported
  • UIAlertView is created in the local settings
  • while creating it, taskinstace class is imported
  • which in turn initializes Sentry
  • which instantiates the executor class
  • which needs settings (but settings are not fully initialized yet so we raise the exception).

@o-nikolas @rkarish you might want to take a look and do some other way of checking if the executor we have supports sentry - without necessarily initializing it. This might be tricky.

@potiuk
Copy link
Member Author

potiuk commented May 21, 2023

BTW. Initializing UI Alert view in local settings is perfectly fine and recommended by us in case anyone asked.

@potiuk
Copy link
Member Author

potiuk commented May 21, 2023

cc: @gil-tober who created the original issue.

As discussed in #31408 (reply in thread) - maybe a good solution could be utilizing listeners on_starting to create the UIAlertView after the initialization is complete. I don't think we want to make it the "only" way though - local_settings is a more "natural" way of doing it, so possibly fixing this circular import might be a good idea anyway.

@gil-tober
Copy link

This is mentioned in #31408
Adding it here for anyone else having this issue

I will try the workaround you suggested and will post my findings.

For now, I found that setting _AIRFLOW__SKIP_DATABASE_EXECUTOR_COMPATIBILITY_CHECK=1 also works as workaround.

@classmethod
@functools.lru_cache(maxsize=None)
def validate_database_executor_compatibility(cls, executor: type[BaseExecutor]) -> None:
"""Validate database and executor compatibility.
Most of the databases work universally, but SQLite can only work with
single-threaded executors (e.g. Sequential).
This is NOT done in ``airflow.configuration`` (when configuration is
initialized) because loading the executor class is heavy work we want to
avoid unless needed.
"""
# Single threaded executors can run with any backend.
if executor.is_single_threaded:
return
# This is set in tests when we want to be able to use SQLite.
if os.environ.get("_AIRFLOW__SKIP_DATABASE_EXECUTOR_COMPATIBILITY_CHECK") == "1":
return
from airflow.settings import engine
# SQLite only works with single threaded executors
if engine.dialect.name == "sqlite":
raise AirflowConfigException(f"error: cannot use SQLite with the {executor.__name__}")

Our metadb is Postgres, so the check for sqlite is not relevant for us

@tanvn
Copy link
Contributor

tanvn commented Apr 8, 2024

I faced a similar issue today with Airflow 2.8.4 (Db migration worked successfully but later, the scheduler and webserver can not start and keep crashing due to the error below)

Our environment:

  • Airflow 2.8.4
  • KubernetesExecutor
  • MySQL 8
  • Sentry enabled
  • airflowLocalSettings in our helm-chart values.yaml contains from airflow.www.utils import UIAlert

Error log from the scheduler

ERROR! Maximum number of retries (20) reached.

Last check result:
$ airflow db check
Failed to import airflow_local_settings.
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/airflow/settings.py", line 500, in import_local_settings
    import airflow_local_settings
  File "/opt/airflow/config/airflow_local_settings.py", line 2, in <module>
    from airflow.www.utils import UIAlert
  File "/usr/local/lib/python3.10/site-packages/airflow/www/utils.py", line 44, in <module>
    from airflow.models.dagrun import DagRun
  File "/usr/local/lib/python3.10/site-packages/airflow/models/dagrun.py", line 59, in <module>
    from airflow.models.taskinstance import TaskInstance as TI
  File "/usr/local/lib/python3.10/site-packages/airflow/models/taskinstance.py", line 99, in <module>
    from airflow.sentry import Sentry
  File "/usr/local/lib/python3.10/site-packages/airflow/sentry.py", line 198, in <module>
    Sentry = ConfiguredSentry()
  File "/usr/local/lib/python3.10/site-packages/airflow/sentry.py", line 90, in __init__
    executor_class, _ = ExecutorLoader.import_default_executor_cls(validate=False)
  File "/usr/local/lib/python3.10/site-packages/airflow/executors/executor_loader.py", line 167, in import_default_executor_cls
    executor, source = cls.import_executor_cls(executor_name, validate=validate)
  File "/usr/local/lib/python3.10/site-packages/airflow/executors/executor_loader.py", line 141, in import_executor_cls
    return _import_and_validate(cls.executors[executor_name]), ConnectorSource.CORE
  File "/usr/local/lib/python3.10/site-packages/airflow/executors/executor_loader.py", line 135, in _import_and_validate
    executor = import_string(path)
  File "/usr/local/lib/python3.10/site-packages/airflow/utils/module_loading.py", line 39, in import_string
    module = import_module(module_path)
  File "/usr/local/lib/python3.10/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "/usr/local/lib/python3.10/site-packages/airflow/providers/cncf/kubernetes/executors/kubernetes_executor.py", line 40, in <module>
    from airflow.providers.cncf.kubernetes.pod_generator import PodMutationHookException, PodReconciliationError
  File "/usr/local/lib/python3.10/site-packages/airflow/providers/cncf/kubernetes/pod_generator.py", line 50, in <module>
    from airflow.providers.cncf.kubernetes.pod_generator_deprecated import (
  File "/usr/local/lib/python3.10/site-packages/airflow/providers/cncf/kubernetes/pod_generator_deprecated.py", line 33, in <module>
    from airflow.utils.hashlib_wrapper import md5
  File "/usr/local/lib/python3.10/site-packages/airflow/utils/hashlib_wrapper.py", line 26, in <module>
    from airflow import PY39
ImportError: cannot import name 'PY39' from partially initialized module 'airflow' (most likely due to a circular import) (/usr/local/lib/python3.10/site-packages/airflow/__init__.py)
Traceback (most recent call last):
  File "/usr/local/bin/airflow", line 5, in <module>
    from airflow.__main__ import main
  File "/usr/local/lib/python3.10/site-packages/airflow/__init__.py", line 68, in <module>
    settings.initialize()
  File "/usr/local/lib/python3.10/site-packages/airflow/settings.py", line 552, in initialize
    import_local_settings()
  File "/usr/local/lib/python3.10/site-packages/airflow/settings.py", line 500, in import_local_settings
    import airflow_local_settings
  File "/opt/airflow/config/airflow_local_settings.py", line 2, in <module>
    from airflow.www.utils import UIAlert
  File "/usr/local/lib/python3.10/site-packages/airflow/www/utils.py", line 44, in <module>
    from airflow.models.dagrun import DagRun
  File "/usr/local/lib/python3.10/site-packages/airflow/models/dagrun.py", line 59, in <module>
    from airflow.models.taskinstance import TaskInstance as TI
  File "/usr/local/lib/python3.10/site-packages/airflow/models/taskinstance.py", line 99, in <module>
    from airflow.sentry import Sentry
  File "/usr/local/lib/python3.10/site-packages/airflow/sentry.py", line 198, in <module>
    Sentry = ConfiguredSentry()
  File "/usr/local/lib/python3.10/site-packages/airflow/sentry.py", line 90, in __init__
    executor_class, _ = ExecutorLoader.import_default_executor_cls(validate=False)
  File "/usr/local/lib/python3.10/site-packages/airflow/executors/executor_loader.py", line 167, in import_default_executor_cls
    executor, source = cls.import_executor_cls(executor_name, validate=validate)
  File "/usr/local/lib/python3.10/site-packages/airflow/executors/executor_loader.py", line 141, in import_executor_cls
    return _import_and_validate(cls.executors[executor_name]), ConnectorSource.CORE
  File "/usr/local/lib/python3.10/site-packages/airflow/executors/executor_loader.py", line 135, in _import_and_validate
    executor = import_string(path)
  File "/usr/local/lib/python3.10/site-packages/airflow/utils/module_loading.py", line 39, in import_string
    module = import_module(module_path)
  File "/usr/local/lib/python3.10/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "/usr/local/lib/python3.10/site-packages/airflow/providers/cncf/kubernetes/executors/kubernetes_executor.py", line 40, in <module>
    from airflow.providers.cncf.kubernetes.pod_generator import PodMutationHookException, PodReconciliationError
  File "/usr/local/lib/python3.10/site-packages/airflow/providers/cncf/kubernetes/pod_generator.py", line 50, in <module>
    from airflow.providers.cncf.kubernetes.pod_generator_deprecated import (
  File "/usr/local/lib/python3.10/site-packages/airflow/providers/cncf/kubernetes/pod_generator_deprecated.py", line 33, in <module>
    from airflow.utils.hashlib_wrapper import md5
  File "/usr/local/lib/python3.10/site-packages/airflow/utils/hashlib_wrapper.py", line 26, in <module>
    from airflow import PY39
ImportError: cannot import name 'PY39' from partially initialized module 'airflow' (most likely due to a circular import) (/usr/local/lib/python3.10/site-packages/airflow/__init__.py)

Setting _AIRFLOW__SKIP_DATABASE_EXECUTOR_COMPATIBILITY_CHECK=1 does not work.

potiuk added a commit to potiuk/airflow that referenced this issue Apr 16, 2024
The import might be invoked when K8S executor starts with sentry on
and it might lead to circular imports

Related: apache#31442
@potiuk
Copy link
Member Author

potiuk commented Apr 16, 2024

I faced a similar issue today with Airflow 2.8.4 (Db migration worked successfully but later, the scheduler and webserver can not start and keep crashing due to the error below)

This is a different issue and likely to be addressed by #39062 - which should be available in the next cncf.kubernetes after merging.

potiuk added a commit that referenced this issue Apr 16, 2024
The import might be invoked when K8S executor starts with sentry on
and it might lead to circular imports

Related: #31442
grrolland pushed a commit to grrolland/airflow that referenced this issue Apr 19, 2024
The import might be invoked when K8S executor starts with sentry on
and it might lead to circular imports

Related: apache#31442
Taragolis pushed a commit that referenced this issue May 6, 2024
…sion (#39056)

* Fixes #36629

* Fixes PR failed test

* Remove an parametrize duplicate tests

* Fix formatting

* Fix formatting

* Fixes #36629

* Fixes PR failed test

* Remove an parametrize duplicate tests

* update simple-salesforce type hints to support 1.12.6 (#39047)

* Fix formatting

* Add changelog for airflow python client 2.9.0 (#39060)

* Upgrade to latest hatchling as build dependency (#39044)

* Prepare docs 1st wave (RC3) + ad hoc April 2024 (#38995) (#39054)

* Prepare docs 1st wave (RC3) + ad hoc April 2024 (#38995)

* update databricks

* [docs] update `DagBag` class docstring to include all params (#38814)

* update docstring for DagBag class

* break long line

* fix space

Signed-off-by: kalyanr <[email protected]>

---------

Signed-off-by: kalyanr <[email protected]>

* Data aware scheduling docs edits (#38687)

* Moves airflow import in deprecated pod_generator to local (#39062)

The import might be invoked when K8S executor starts with sentry on
and it might lead to circular imports

Related: #31442

* KPO xcom sidecar PodDefault usage (#38951)

We should use the same, non deprecated, version of PodDefaults for the
xcom sidecar when creating and reading xcom.

* Fix formatting

* Change date/time parsing method for newer_than parameter un SFTPSensor

* Add examples in AWS auth manager documentation (#39040)

* update document (#39068)

* Update hatchling to version 1.24.0 (#39072)

* Check that the dataset<>task exists before trying to render graph (#39069)

* Change date/time parsing method for newer_than parameter un SFTPSensor

* Fix utc timezone in unit tests

* Fix utc timezone in unit tests

---------

Signed-off-by: kalyanr <[email protected]>
Co-authored-by: Grégoire Rolland <[email protected]>
Co-authored-by: Hussein Awala <[email protected]>
Co-authored-by: Ephraim Anierobi <[email protected]>
Co-authored-by: Jarek Potiuk <[email protected]>
Co-authored-by: Elad Kalif <[email protected]>
Co-authored-by: Kalyan <[email protected]>
Co-authored-by: Laura Zdanski <[email protected]>
Co-authored-by: Jed Cunningham <[email protected]>
Co-authored-by: Vincent <[email protected]>
Co-authored-by: humit <[email protected]>
Co-authored-by: Brent Bovenzi <[email protected]>
pateash pushed a commit to pateash/airflow that referenced this issue May 13, 2024
…sion (apache#39056)

* Fixes apache#36629

* Fixes PR failed test

* Remove an parametrize duplicate tests

* Fix formatting

* Fix formatting

* Fixes apache#36629

* Fixes PR failed test

* Remove an parametrize duplicate tests

* update simple-salesforce type hints to support 1.12.6 (apache#39047)

* Fix formatting

* Add changelog for airflow python client 2.9.0 (apache#39060)

* Upgrade to latest hatchling as build dependency (apache#39044)

* Prepare docs 1st wave (RC3) + ad hoc April 2024 (apache#38995) (apache#39054)

* Prepare docs 1st wave (RC3) + ad hoc April 2024 (apache#38995)

* update databricks

* [docs] update `DagBag` class docstring to include all params (apache#38814)

* update docstring for DagBag class

* break long line

* fix space

Signed-off-by: kalyanr <[email protected]>

---------

Signed-off-by: kalyanr <[email protected]>

* Data aware scheduling docs edits (apache#38687)

* Moves airflow import in deprecated pod_generator to local (apache#39062)

The import might be invoked when K8S executor starts with sentry on
and it might lead to circular imports

Related: apache#31442

* KPO xcom sidecar PodDefault usage (apache#38951)

We should use the same, non deprecated, version of PodDefaults for the
xcom sidecar when creating and reading xcom.

* Fix formatting

* Change date/time parsing method for newer_than parameter un SFTPSensor

* Add examples in AWS auth manager documentation (apache#39040)

* update document (apache#39068)

* Update hatchling to version 1.24.0 (apache#39072)

* Check that the dataset<>task exists before trying to render graph (apache#39069)

* Change date/time parsing method for newer_than parameter un SFTPSensor

* Fix utc timezone in unit tests

* Fix utc timezone in unit tests

---------

Signed-off-by: kalyanr <[email protected]>
Co-authored-by: Grégoire Rolland <[email protected]>
Co-authored-by: Hussein Awala <[email protected]>
Co-authored-by: Ephraim Anierobi <[email protected]>
Co-authored-by: Jarek Potiuk <[email protected]>
Co-authored-by: Elad Kalif <[email protected]>
Co-authored-by: Kalyan <[email protected]>
Co-authored-by: Laura Zdanski <[email protected]>
Co-authored-by: Jed Cunningham <[email protected]>
Co-authored-by: Vincent <[email protected]>
Co-authored-by: humit <[email protected]>
Co-authored-by: Brent Bovenzi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected_version:2.6 Issues Reported for 2.6 area:core kind:bug This is a clearly a bug
Projects
None yet
Development

No branches or pull requests

4 participants