From b62220285c625e0e5322d8e9a5d206d5b577a149 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Sat, 16 Nov 2024 03:58:13 +0000 Subject: [PATCH] Do not reset DB in CI tests if not needed The #43979 refactoring of tests caused unnecessary database reset attempts in tests that did not require it or had no database set. This caused unnecessary `airflow db reset` in collection-only tests with removed non-ARM packages, but also it caused the error printed in non-DB tests: ``` Resetting the DB [2024-11-16T03:50:37.812+0000] {cli_parser.py:67} ERROR - Failed to load CLI commands from executor: LocalExecutor Traceback (most recent call last): File "/opt/airflow/airflow/cli/cli_parser.py", line 64, in executor, _ = ExecutorLoader.import_executor_cls(executor_name) File "/opt/airflow/airflow/executors/executor_loader.py", line 285, in import_executor_cls return _import_and_validate(executor_name.module_path), executor_name.connector_source File "/opt/airflow/airflow/executors/executor_loader.py", line 282, in _import_and_validate cls.validate_database_executor_compatibility(executor) File "/opt/airflow/airflow/executors/executor_loader.py", line 327, in validate_database_executor_compatibility if engine.dialect.name == "sqlite": AttributeError: 'NoneType' object has no attribute 'dialect' [2024-11-16T03:50:37.813+0000] {cli_parser.py:68} ERROR - Ensure all dependencies are met and try again. If using a Celery based executor install a 3.3.0+ version of the Celery provider. If using a Kubernetes executor, install a 7.4.0+ version of the CNCF provider Traceback (most recent call last): File "/usr/local/bin/airflow", line 8, in sys.exit(main()) File "/opt/airflow/airflow/__main__.py", line 62, in main args.func(args) File "/opt/airflow/airflow/cli/cli_config.py", line 49, in command return func(*args, **kwargs) File "/opt/airflow/airflow/utils/providers_configuration_loader.py", line 55, in wrapped_function return func(*args, **kwargs) File "/opt/airflow/airflow/cli/commands/db_command.py", line 63, in resetdb print(f"DB: {settings.engine.url!r}") AttributeError: 'NoneType' object has no attribute 'url' Database has been reset ``` The fix is to add `--no-db-reset` in collection tests and force db_reset = False in case `skip_db_tests` is set to True. --- dev/breeze/doc/images/output-commands.svg | 16 +- dev/breeze/doc/images/output_shell.svg | 16 +- dev/breeze/doc/images/output_shell.txt | 2 +- .../doc/images/output_start-airflow.svg | 16 +- .../doc/images/output_start-airflow.txt | 2 +- .../output_testing_core-integration-tests.svg | 102 ++++----- .../output_testing_core-integration-tests.txt | 2 +- .../doc/images/output_testing_core-tests.svg | 182 ++++++++-------- .../doc/images/output_testing_core-tests.txt | 2 +- ...ut_testing_providers-integration-tests.svg | 104 ++++----- ...ut_testing_providers-integration-tests.txt | 2 +- .../images/output_testing_providers-tests.svg | 206 +++++++++--------- .../images/output_testing_providers-tests.txt | 2 +- .../images/output_testing_system-tests.svg | 94 ++++---- .../images/output_testing_system-tests.txt | 2 +- .../airflow_breeze/commands/common_options.py | 4 +- .../commands/testing_commands.py | 2 +- scripts/ci/testing/run_unit_tests.sh | 4 +- 18 files changed, 391 insertions(+), 369 deletions(-) diff --git a/dev/breeze/doc/images/output-commands.svg b/dev/breeze/doc/images/output-commands.svg index feee4e22b768f..77c77525301c7 100644 --- a/dev/breeze/doc/images/output-commands.svg +++ b/dev/breeze/doc/images/output-commands.svg @@ -321,14 +321,14 @@ │(TEXT)                                                                                            │ ╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ ╭─ Database ───────────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│--backend-bDatabase backend to use. If 'none' is chosen, Breeze will start with an invalid database    │ -│configuration, meaning there will be no database available, and any attempts to connect to  │ -│the Airflow database will fail.                                                             │ -│(>sqlite< | mysql | postgres | none)                                                        │ -│[default: sqlite]                                                                           │ -│--postgres-version-PVersion of Postgres used.(>12< | 13 | 14 | 15 | 16 | 17)[default: 12]│ -│--mysql-version-MVersion of MySQL used.(>8.0< | 8.4)[default: 8.0]│ -│--db-reset-dReset DB when entering the container.│ +│--backend-bDatabase backend to use. If 'none' is chosen, Breeze will start with an       │ +│invalid database configuration, meaning there will be no database available,  │ +│and any attempts to connect to the Airflow database will fail.                │ +│(>sqlite< | mysql | postgres | none)                                          │ +│[default: sqlite]                                                             │ +│--postgres-version-PVersion of Postgres used.(>12< | 13 | 14 | 15 | 16 | 17)[default: 12]│ +│--mysql-version-MVersion of MySQL used.(>8.0< | 8.4)[default: 8.0]│ +│--db-reset-d/--no-db-resetReset DB when entering the container.[default: no-db-reset]│ ╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ ╭─ Build CI image (before entering shell) ─────────────────────────────────────────────────────────────────────────────╮ │--github-repository-gGitHub repository used to pull, push run images.(TEXT)[default: apache/airflow]│ diff --git a/dev/breeze/doc/images/output_shell.svg b/dev/breeze/doc/images/output_shell.svg index f77b3dd907e74..7c126fbef524f 100644 --- a/dev/breeze/doc/images/output_shell.svg +++ b/dev/breeze/doc/images/output_shell.svg @@ -541,14 +541,14 @@ │[default: auto]                                                                │ ╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ ╭─ Database ───────────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│--backend-bDatabase backend to use. If 'none' is chosen, Breeze will start with an invalid database    │ -│configuration, meaning there will be no database available, and any attempts to connect to  │ -│the Airflow database will fail.                                                             │ -│(>sqlite< | mysql | postgres | none)                                                        │ -│[default: sqlite]                                                                           │ -│--postgres-version-PVersion of Postgres used.(>12< | 13 | 14 | 15 | 16 | 17)[default: 12]│ -│--mysql-version-MVersion of MySQL used.(>8.0< | 8.4)[default: 8.0]│ -│--db-reset-dReset DB when entering the container.│ +│--backend-bDatabase backend to use. If 'none' is chosen, Breeze will start with an       │ +│invalid database configuration, meaning there will be no database available,  │ +│and any attempts to connect to the Airflow database will fail.                │ +│(>sqlite< | mysql | postgres | none)                                          │ +│[default: sqlite]                                                             │ +│--postgres-version-PVersion of Postgres used.(>12< | 13 | 14 | 15 | 16 | 17)[default: 12]│ +│--mysql-version-MVersion of MySQL used.(>8.0< | 8.4)[default: 8.0]│ +│--db-reset-d/--no-db-resetReset DB when entering the container.[default: no-db-reset]│ ╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ ╭─ Choose executor ────────────────────────────────────────────────────────────────────────────────────────────────────╮ │--executorSpecify the executor to use with shell command.                                                  │ diff --git a/dev/breeze/doc/images/output_shell.txt b/dev/breeze/doc/images/output_shell.txt index e25411b643e1c..073e8cdc87240 100644 --- a/dev/breeze/doc/images/output_shell.txt +++ b/dev/breeze/doc/images/output_shell.txt @@ -1 +1 @@ -706a617a798c2d5a0f64d356cab3ef96 +369acf181f53ebddbaf928f4743e0014 diff --git a/dev/breeze/doc/images/output_start-airflow.svg b/dev/breeze/doc/images/output_start-airflow.svg index 7dd09a1920e27..d7295570c013f 100644 --- a/dev/breeze/doc/images/output_start-airflow.svg +++ b/dev/breeze/doc/images/output_start-airflow.svg @@ -427,14 +427,14 @@ │(TEXT)                                                                                │ ╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ ╭─ Database ───────────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│--backend-bDatabase backend to use. If 'none' is chosen, Breeze will start with an invalid database    │ -│configuration, meaning there will be no database available, and any attempts to connect to  │ -│the Airflow database will fail.                                                             │ -│(>sqlite< | mysql | postgres | none)                                                        │ -│[default: sqlite]                                                                           │ -│--postgres-version-PVersion of Postgres used.(>12< | 13 | 14 | 15 | 16 | 17)[default: 12]│ -│--mysql-version-MVersion of MySQL used.(>8.0< | 8.4)[default: 8.0]│ -│--db-reset-dReset DB when entering the container.│ +│--backend-bDatabase backend to use. If 'none' is chosen, Breeze will start with an       │ +│invalid database configuration, meaning there will be no database available,  │ +│and any attempts to connect to the Airflow database will fail.                │ +│(>sqlite< | mysql | postgres | none)                                          │ +│[default: sqlite]                                                             │ +│--postgres-version-PVersion of Postgres used.(>12< | 13 | 14 | 15 | 16 | 17)[default: 12]│ +│--mysql-version-MVersion of MySQL used.(>8.0< | 8.4)[default: 8.0]│ +│--db-reset-d/--no-db-resetReset DB when entering the container.[default: no-db-reset]│ ╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ ╭─ Choosing executor ──────────────────────────────────────────────────────────────────────────────────────────────────╮ │--executorSpecify the executor to use with start-airflow (defaults to LocalExecutor or CeleryExecutor      │ diff --git a/dev/breeze/doc/images/output_start-airflow.txt b/dev/breeze/doc/images/output_start-airflow.txt index 9518daef474fa..10f07b73a0366 100644 --- a/dev/breeze/doc/images/output_start-airflow.txt +++ b/dev/breeze/doc/images/output_start-airflow.txt @@ -1 +1 @@ -6821f495c57914229fd83e5ab850e38f +3c833010b2dc87dd333e69700a1b7073 diff --git a/dev/breeze/doc/images/output_testing_core-integration-tests.svg b/dev/breeze/doc/images/output_testing_core-integration-tests.svg index 5128d8b830909..95f22dfedd5f5 100644 --- a/dev/breeze/doc/images/output_testing_core-integration-tests.svg +++ b/dev/breeze/doc/images/output_testing_core-integration-tests.svg @@ -1,4 +1,4 @@ - + Run the specified integration tests. ╭─ Test options ───────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│--test-timeoutTest timeout in seconds. Set the pytest setup, execution and teardown timeouts to this value│ -│(INTEGER RANGE)                                                                             │ -│[default: 60; x>=0]                                                                         │ -│--enable-coverageEnable coverage capturing for tests in the form of XML files│ -│--collect-onlyCollect tests only, do not run them.│ -│--db-reset-dReset DB when entering the container.│ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ -╭─ Test environment ───────────────────────────────────────────────────────────────────────────────────────────────────╮ -│--backend-bDatabase backend to use. If 'none' is chosen, Breeze will start with│ -│an invalid database configuration, meaning there will be no database│ -│available, and any attempts to connect to the Airflow database will │ -│fail.                                                               │ -│(>sqlite< | mysql | postgres | none)                                │ -│[default: sqlite]                                                   │ -│--no-db-cleanupDo not clear the database before each test module│ -│--python-pPython major/minor version used in Airflow image for images.│ -│(>3.9< | 3.10 | 3.11 | 3.12)                                │ -│[default: 3.9]                                              │ -│--postgres-version-PVersion of Postgres used.(>12< | 13 | 14 | 15 | 16 | 17)│ -│[default: 12]            │ -│--mysql-version-MVersion of MySQL used.(>8.0< | 8.4)[default: 8.0]│ -│--forward-credentials-fForward local credentials to container when running.│ -│--force-sa-warnings/--no-force-sa-warningsEnable `sqlalchemy.exc.MovedIn20Warning` during the tests runs.│ -│[default: force-sa-warnings]                                   │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ -╭─ Integration tests ──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│--integrationCore Integrations to enable when running (can be more than one).   │ -│(all | all-testable | celery | kerberos | keycloak | otel | statsd)│ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ -╭─ Advanced flag for tests command ────────────────────────────────────────────────────────────────────────────────────╮ -│--github-repository-gGitHub repository used to pull, push run images.(TEXT)[default: apache/airflow]│ -│--image-tagTag of the image which is used to run the image (implies --mount-sources=skip).│ -│(TEXT)                                                                         │ -│[default: latest]                                                              │ -│--mount-sourcesChoose scope of local sources that should be mounted, skipped, or removed (default =│ -│selected).                                                                          │ -│(selected | all | skip | remove | tests | providers-and-tests)                      │ -│[default: selected]                                                                 │ -│--skip-docker-compose-downSkips running docker-compose down after tests│ -│--keep-env-variablesDo not clear environment variables that might have side effect while running tests│ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ -╭─ Common options ─────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│--dry-run-DIf dry-run is set, commands are only printed, not executed.│ -│--verbose-vPrint verbose information about performed steps.│ -│--help-hShow this message and exit.│ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ +│--test-timeoutTest timeout in seconds. Set the pytest setup, execution and teardown timeouts │ +│to this value                                                                  │ +│(INTEGER RANGE)                                                                │ +│[default: 60; x>=0]                                                            │ +│--enable-coverageEnable coverage capturing for tests in the form of XML files│ +│--collect-onlyCollect tests only, do not run them.│ +│--db-reset-d/--no-db-resetReset DB when entering the container.[default: no-db-reset]│ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ +╭─ Test environment ───────────────────────────────────────────────────────────────────────────────────────────────────╮ +│--backend-bDatabase backend to use. If 'none' is chosen, Breeze will start with│ +│an invalid database configuration, meaning there will be no database│ +│available, and any attempts to connect to the Airflow database will │ +│fail.                                                               │ +│(>sqlite< | mysql | postgres | none)                                │ +│[default: sqlite]                                                   │ +│--no-db-cleanupDo not clear the database before each test module│ +│--python-pPython major/minor version used in Airflow image for images.│ +│(>3.9< | 3.10 | 3.11 | 3.12)                                │ +│[default: 3.9]                                              │ +│--postgres-version-PVersion of Postgres used.(>12< | 13 | 14 | 15 | 16 | 17)│ +│[default: 12]            │ +│--mysql-version-MVersion of MySQL used.(>8.0< | 8.4)[default: 8.0]│ +│--forward-credentials-fForward local credentials to container when running.│ +│--force-sa-warnings/--no-force-sa-warningsEnable `sqlalchemy.exc.MovedIn20Warning` during the tests runs.│ +│[default: force-sa-warnings]                                   │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ +╭─ Integration tests ──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│--integrationCore Integrations to enable when running (can be more than one).   │ +│(all | all-testable | celery | kerberos | keycloak | otel | statsd)│ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ +╭─ Advanced flag for tests command ────────────────────────────────────────────────────────────────────────────────────╮ +│--github-repository-gGitHub repository used to pull, push run images.(TEXT)[default: apache/airflow]│ +│--image-tagTag of the image which is used to run the image (implies --mount-sources=skip).│ +│(TEXT)                                                                         │ +│[default: latest]                                                              │ +│--mount-sourcesChoose scope of local sources that should be mounted, skipped, or removed (default =│ +│selected).                                                                          │ +│(selected | all | skip | remove | tests | providers-and-tests)                      │ +│[default: selected]                                                                 │ +│--skip-docker-compose-downSkips running docker-compose down after tests│ +│--keep-env-variablesDo not clear environment variables that might have side effect while running tests│ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ +╭─ Common options ─────────────────────────────────────────────────────────────────────────────────────────────────────╮ +│--dry-run-DIf dry-run is set, commands are only printed, not executed.│ +│--verbose-vPrint verbose information about performed steps.│ +│--help-hShow this message and exit.│ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ diff --git a/dev/breeze/doc/images/output_testing_core-integration-tests.txt b/dev/breeze/doc/images/output_testing_core-integration-tests.txt index 462811c88cf02..eb69a8e519869 100644 --- a/dev/breeze/doc/images/output_testing_core-integration-tests.txt +++ b/dev/breeze/doc/images/output_testing_core-integration-tests.txt @@ -1 +1 @@ -7eb5ce3a9b1e3aa95bfe0bc66f2ba06a +5894cf887269f4cc5da2b0c5fb015685 diff --git a/dev/breeze/doc/images/output_testing_core-tests.svg b/dev/breeze/doc/images/output_testing_core-tests.svg index 01a95c334c69b..8f3f7f03eb7e2 100644 --- a/dev/breeze/doc/images/output_testing_core-tests.svg +++ b/dev/breeze/doc/images/output_testing_core-tests.svg @@ -1,4 +1,4 @@ - +