apache / airflow

Apache Airflow - A platform to programmatically author, schedule, and monitor workflows
https://airflow.apache.org/
Apache License 2.0
36.85k stars 14.25k forks source link

Adding UIAlert in local settings fails with Sentry enabled #31442

Open potiuk opened 1 year ago

potiuk commented 1 year ago

Discussed in https://github.com/apache/airflow/discussions/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 from airflow.__main__ import main File “/home/airflow/.local/lib/python3.10/site-packages/airflow/__init__.py”, line 66, in 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 from airflow.www.utils import UIAlert File “/home/airflow/.local/lib/python3.10/site-packages/airflow/www/utils.py”, line 44, in from airflow.models.dagrun import DagRun File “/home/airflow/.local/lib/python3.10/site-packages/airflow/models/dagrun.py”, line 57, in from airflow.models.taskinstance import TaskInstance as TI File “/home/airflow/.local/lib/python3.10/site-packages/airflow/models/taskinstance.py”, line 100, in from airflow.sentry import Sentry File “/home/airflow/.local/lib/python3.10/site-packages/airflow/sentry.py”, line 195, in 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 - [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
potiuk commented 1 year ago

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

potiuk commented 1 year ago

Very interesting bug :) .

The root cause is AIP-51 related change https://github.com/apache/airflow/pull/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:

@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 commented 1 year ago

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

potiuk commented 1 year ago

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

As discussed in https://github.com/apache/airflow/discussions/31408#discussioncomment-5958636 - 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 commented 1 year ago

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.

https://github.com/apache/airflow/blob/9137740b2e8937aaf65c7c86dd11b096d028e7d2/airflow/executors/executor_loader.py#L160-L186

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

tanvn commented 6 months ago

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:

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 commented 6 months ago

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 https://github.com/apache/airflow/pull/39062 - which should be available in the next cncf.kubernetes after merging.