Skip to content

Commit

Permalink
Quotes the components of a PostgreSQL connection URL when built from …
Browse files Browse the repository at this point in the history
…components (#15094)
  • Loading branch information
chrisguidry authored Aug 27, 2024
1 parent 176b306 commit 4524a63
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 31 deletions.
21 changes: 14 additions & 7 deletions src/prefect/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -425,13 +425,20 @@ def default_database_connection_url(settings: "Settings", value: Optional[str]):
f"Missing required database connection settings: {', '.join(missing)}"
)

host = PREFECT_API_DATABASE_HOST.value_from(settings)
port = PREFECT_API_DATABASE_PORT.value_from(settings) or 5432
user = PREFECT_API_DATABASE_USER.value_from(settings)
name = PREFECT_API_DATABASE_NAME.value_from(settings)
password = PREFECT_API_DATABASE_PASSWORD.value_from(settings)

return f"{driver}://{user}:{password}@{host}:{port}/{name}"
# We only need SQLAlchemy here if we're parsing a remote database connection
# string. Import it here so that we don't require the prefect-client package
# to have SQLAlchemy installed.
from sqlalchemy import URL

return URL(
drivername=driver,
host=PREFECT_API_DATABASE_HOST.value_from(settings),
port=PREFECT_API_DATABASE_PORT.value_from(settings) or 5432,
username=PREFECT_API_DATABASE_USER.value_from(settings),
password=PREFECT_API_DATABASE_PASSWORD.value_from(settings),
database=PREFECT_API_DATABASE_NAME.value_from(settings),
query=[],
).render_as_string(hide_password=False)

elif driver == "sqlite+aiosqlite":
path = PREFECT_API_DATABASE_NAME.value_from(settings)
Expand Down
72 changes: 48 additions & 24 deletions tests/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import pydantic
import pytest
from sqlalchemy import make_url

import prefect.context
import prefect.settings
Expand Down Expand Up @@ -511,12 +512,33 @@ def test_postgres_database_settings_may_be_set_individually(self):
PREFECT_API_DATABASE_PASSWORD: "the-password",
}
):
assert PREFECT_API_DATABASE_CONNECTION_URL.value() == (
"postgresql+asyncpg://"
"the-user:the-password@"
"the-database-server.example.com:15432"
"/the-database"
)
url = make_url(PREFECT_API_DATABASE_CONNECTION_URL.value())
assert url.drivername == "postgresql+asyncpg"
assert url.host == "the-database-server.example.com"
assert url.port == 15432
assert url.username == "the-user"
assert url.database == "the-database"
assert url.password == "the-password"

def test_postgres_password_is_quoted(self):
with temporary_settings(
{
PREFECT_API_DATABASE_CONNECTION_URL: None,
PREFECT_API_DATABASE_DRIVER: "postgresql+asyncpg",
PREFECT_API_DATABASE_HOST: "the-database-server.example.com",
PREFECT_API_DATABASE_PORT: 15432,
PREFECT_API_DATABASE_USER: "the-user",
PREFECT_API_DATABASE_NAME: "the-database",
PREFECT_API_DATABASE_PASSWORD: "the-password:has:funky!@stuff",
}
):
url = make_url(PREFECT_API_DATABASE_CONNECTION_URL.value())
assert url.drivername == "postgresql+asyncpg"
assert url.host == "the-database-server.example.com"
assert url.port == 15432
assert url.username == "the-user"
assert url.database == "the-database"
assert url.password == "the-password:has:funky!@stuff"

def test_postgres_database_settings_defaults_port(self):
with temporary_settings(
Expand All @@ -529,12 +551,13 @@ def test_postgres_database_settings_defaults_port(self):
PREFECT_API_DATABASE_PASSWORD: "the-password",
}
):
assert PREFECT_API_DATABASE_CONNECTION_URL.value() == (
"postgresql+asyncpg://"
"the-user:the-password@"
"the-database-server.example.com:5432"
"/the-database"
)
url = make_url(PREFECT_API_DATABASE_CONNECTION_URL.value())
assert url.drivername == "postgresql+asyncpg"
assert url.host == "the-database-server.example.com"
assert url.port == 5432
assert url.username == "the-user"
assert url.database == "the-database"
assert url.password == "the-password"

def test_sqlite_database_settings_may_be_set_individually(self):
with temporary_settings(
Expand All @@ -544,9 +567,9 @@ def test_sqlite_database_settings_may_be_set_individually(self):
PREFECT_API_DATABASE_NAME: "/the/database/file/path.db",
}
):
assert PREFECT_API_DATABASE_CONNECTION_URL.value() == (
"sqlite+aiosqlite:////the/database/file/path.db"
)
url = make_url(PREFECT_API_DATABASE_CONNECTION_URL.value())
assert url.drivername == "sqlite+aiosqlite"
assert url.database == "/the/database/file/path.db"

def test_sqlite_database_driver_uses_default_path(self):
with temporary_settings(
Expand All @@ -555,9 +578,9 @@ def test_sqlite_database_driver_uses_default_path(self):
PREFECT_API_DATABASE_DRIVER: "sqlite+aiosqlite",
}
):
assert PREFECT_API_DATABASE_CONNECTION_URL.value() == (
f"sqlite+aiosqlite:///{PREFECT_HOME.value()}/prefect.db"
)
url = make_url(PREFECT_API_DATABASE_CONNECTION_URL.value())
assert url.drivername == "sqlite+aiosqlite"
assert url.database == f"{PREFECT_HOME.value()}/prefect.db"

def test_unknown_driver_raises(self):
with pytest.raises(pydantic.ValidationError, match="literal_error"):
Expand Down Expand Up @@ -587,12 +610,13 @@ def test_connection_string_with_dollar_sign(self):
PREFECT_API_DATABASE_USER: "the-user",
}
):
assert PREFECT_API_DATABASE_CONNECTION_URL.value() == (
"postgresql+asyncpg://"
"the-user:the-$password@"
"the-database-server.example.com:5432"
"/the-database"
)
url = make_url(PREFECT_API_DATABASE_CONNECTION_URL.value())
assert url.drivername == "postgresql+asyncpg"
assert url.host == "the-database-server.example.com"
assert url.port == 5432
assert url.username == "the-user"
assert url.database == "the-database"
assert url.password == "the-$password"


class TestTemporarySettings:
Expand Down

0 comments on commit 4524a63

Please sign in to comment.