Skip to content

Commit

Permalink
Support checking for db path absoluteness on Windows (apache#40069)
Browse files Browse the repository at this point in the history
  • Loading branch information
Dev-iL authored and romsharon98 committed Jul 26, 2024
1 parent 8e9ddd3 commit 63aa9e6
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 12 deletions.
27 changes: 20 additions & 7 deletions airflow/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,17 +278,30 @@ def remove(*args, **kwargs):
pass


def _is_sqlite_db_path_relative(sqla_conn_str: str) -> bool:
"""Determine whether the database connection URI specifies a relative path."""
# Check for non-empty connection string:
if not sqla_conn_str:
return False
# Check for the right URI scheme:
if not sqla_conn_str.startswith("sqlite"):
return False
# In-memory is not useful for production, but useful for writing tests against Airflow for extensions
if sqla_conn_str == "sqlite://":
return False
# Check for absolute path:
if sqla_conn_str.startswith(abs_prefix := "sqlite:///") and os.path.isabs(
sqla_conn_str[len(abs_prefix) :]
):
return False
return True


def configure_orm(disable_connection_pool=False, pool_class=None):
"""Configure ORM using SQLAlchemy."""
from airflow.utils.log.secrets_masker import mask_secret

if (
SQL_ALCHEMY_CONN
and SQL_ALCHEMY_CONN.startswith("sqlite")
and not SQL_ALCHEMY_CONN.startswith("sqlite:////")
# In memory is not useful for production, but useful for writing tests against Airflow for extensions
and SQL_ALCHEMY_CONN != "sqlite://"
):
if _is_sqlite_db_path_relative(SQL_ALCHEMY_CONN):
from airflow.exceptions import AirflowConfigException

raise AirflowConfigException(
Expand Down
3 changes: 2 additions & 1 deletion airflow/www/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import warnings
from datetime import timedelta
from os.path import isabs

from flask import Flask
from flask_appbuilder import SQLA
Expand Down Expand Up @@ -100,7 +101,7 @@ def create_app(config=None, testing=False):
flask_app.config["REQUIRE_CONFIRMATION_DAG_CHANGE"] = require_confirmation_dag_change

url = make_url(flask_app.config["SQLALCHEMY_DATABASE_URI"])
if url.drivername == "sqlite" and url.database and not url.database.startswith("/"):
if url.drivername == "sqlite" and url.database and not isabs(url.database):
raise AirflowConfigException(
f'Cannot use relative path: `{conf.get("database", "SQL_ALCHEMY_CONN")}` to connect to sqlite. '
"Please use absolute path such as `sqlite:////tmp/airflow.db`."
Expand Down
18 changes: 14 additions & 4 deletions tests/core/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,14 +230,24 @@ def test_uses_updated_session_timeout_config_by_default(self):
assert session_lifetime_config == default_timeout_minutes


_local_db_path_error = pytest.raises(AirflowConfigException, match=r"Cannot use relative path:")


@pytest.mark.parametrize(
["value", "expectation"],
[
(
"sqlite:///./relative_path.db",
pytest.raises(AirflowConfigException, match=r"Cannot use relative path:"),
("sqlite:///./relative_path.db", _local_db_path_error),
("sqlite:///relative/path.db", _local_db_path_error),
pytest.param(
"sqlite:///C:/path/to/db",
_local_db_path_error,
marks=pytest.mark.skipif(sys.platform.startswith("win"), reason="Skip on Windows"),
),
pytest.param(
r"sqlite:///C:\path\to\db",
_local_db_path_error,
marks=pytest.mark.skipif(sys.platform.startswith("win"), reason="Skip on Windows"),
),
# Should not raise an exception
("sqlite://", contextlib.nullcontext()),
],
)
Expand Down

0 comments on commit 63aa9e6

Please sign in to comment.