From 15141549363dd55a52f2eae77ea006183aba5ab2 Mon Sep 17 00:00:00 2001 From: vincentsarago Date: Tue, 13 May 2025 23:51:23 +0200 Subject: [PATCH 1/8] refactor read/write connection pool --- .github/workflows/cicd.yaml | 3 +- CHANGES.md | 14 +++++ docker-compose.yml | 3 +- stac_fastapi/pgstac/app.py | 9 ++- stac_fastapi/pgstac/config.py | 20 ++----- stac_fastapi/pgstac/db.py | 72 ++++++++++++------------ stac_fastapi/pgstac/extensions/filter.py | 4 +- tests/api/test_api.py | 20 +++++-- tests/clients/test_postgres.py | 6 +- tests/conftest.py | 34 +++++++---- tests/resources/test_mgmt.py | 3 +- 11 files changed, 102 insertions(+), 86 deletions(-) diff --git a/.github/workflows/cicd.yaml b/.github/workflows/cicd.yaml index 17162aff..682e2c31 100644 --- a/.github/workflows/cicd.yaml +++ b/.github/workflows/cicd.yaml @@ -93,8 +93,7 @@ jobs: POSTGRES_USER: username POSTGRES_PASS: password POSTGRES_DBNAME: postgis - POSTGRES_HOST_READER: localhost - POSTGRES_HOST_WRITER: localhost + POSTGRES_HOST: localhost POSTGRES_PORT: 5432 PGUSER: username PGPASSWORD: password diff --git a/CHANGES.md b/CHANGES.md index bdc2ea68..ca03ba9c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,11 +4,25 @@ ### Changed +- rename `POSTGRES_HOST_READER` to `POSTGRES_HOST` in config **breaking change** +- rename `reader_connection_string` to `connection_string` in `PostgresSettings` class **breaking change** - add `ENABLE_TRANSACTIONS_EXTENSIONS` env variable to enable `transaction` extensions - disable transaction and bulk_transactions extensions by default **breaking change** - update `stac-fastapi-*` version requirements to `>=5.2,<6.0` - add pgstac health-check in `/_mgmt/health` +### Added + +- add `write_connection_pool` option in `stac_fastapi.pgstac.db.connect_to_db` function +- add `write_postgres_settings` option in `stac_fastapi.pgstac.db.connect_to_db` function to set specific settings for the `writer` DB connection pool + +### removed + +- `stac_fastapi.pgstac.db.DB` class +- `POSTGRES_HOST_WRITER` in config +- `writer_connection_string` in `PostgresSettings` class +- `testing_connection_string` in `PostgresSettings` class + ## [5.0.2] - 2025-04-07 ### Fixed diff --git a/docker-compose.yml b/docker-compose.yml index 962aa2ee..b0ce3462 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -11,8 +11,7 @@ services: - POSTGRES_USER=username - POSTGRES_PASS=password - POSTGRES_DBNAME=postgis - - POSTGRES_HOST_READER=database - - POSTGRES_HOST_WRITER=database + - POSTGRES_HOST=database - POSTGRES_PORT=5432 - WEB_CONCURRENCY=10 - VSI_CACHE=TRUE diff --git a/stac_fastapi/pgstac/app.py b/stac_fastapi/pgstac/app.py index d19a71ec..5d42f769 100644 --- a/stac_fastapi/pgstac/app.py +++ b/stac_fastapi/pgstac/app.py @@ -95,7 +95,12 @@ application_extensions = [] -if os.environ.get("ENABLE_TRANSACTIONS_EXTENSIONS", "").lower() in ["yes", "true", "1"]: +with_transactions = os.environ.get("ENABLE_TRANSACTIONS_EXTENSIONS", "").lower() in [ + "yes", + "true", + "1", +] +if with_transactions: application_extensions.append( TransactionExtension( client=TransactionsClient(), @@ -150,7 +155,7 @@ @asynccontextmanager async def lifespan(app: FastAPI): """FastAPI Lifespan.""" - await connect_to_db(app) + await connect_to_db(app, add_write_connection_pool=with_transactions) yield await close_db_connection(app) diff --git a/stac_fastapi/pgstac/config.py b/stac_fastapi/pgstac/config.py index e1cd3b46..ea21709d 100644 --- a/stac_fastapi/pgstac/config.py +++ b/stac_fastapi/pgstac/config.py @@ -54,8 +54,7 @@ class PostgresSettings(BaseSettings): Attributes: postgres_user: postgres username. postgres_pass: postgres password. - postgres_host_reader: hostname for the reader connection. - postgres_host_writer: hostname for the writer connection. + postgres_host: hostname for the connection. postgres_port: database port. postgres_dbname: database name. use_api_hydrate: perform hydration of stac items within stac-fastapi. @@ -64,8 +63,7 @@ class PostgresSettings(BaseSettings): postgres_user: str postgres_pass: str - postgres_host_reader: str - postgres_host_writer: str + postgres_host: str postgres_port: int postgres_dbname: str @@ -79,19 +77,9 @@ class PostgresSettings(BaseSettings): model_config = {"env_file": ".env", "extra": "ignore"} @property - def reader_connection_string(self): + def connection_string(self): """Create reader psql connection string.""" - return f"postgresql://{self.postgres_user}:{quote(self.postgres_pass)}@{self.postgres_host_reader}:{self.postgres_port}/{self.postgres_dbname}" - - @property - def writer_connection_string(self): - """Create writer psql connection string.""" - return f"postgresql://{self.postgres_user}:{quote(self.postgres_pass)}@{self.postgres_host_writer}:{self.postgres_port}/{self.postgres_dbname}" - - @property - def testing_connection_string(self): - """Create testing psql connection string.""" - return f"postgresql://{self.postgres_user}:{quote(self.postgres_pass)}@{self.postgres_host_writer}:{self.postgres_port}/pgstactestdb" + return f"postgresql://{self.postgres_user}:{quote(self.postgres_pass)}@{self.postgres_host}:{self.postgres_port}/{self.postgres_dbname}" class Settings(ApiSettings): diff --git a/stac_fastapi/pgstac/db.py b/stac_fastapi/pgstac/db.py index e0e1420a..4aa48bd2 100644 --- a/stac_fastapi/pgstac/db.py +++ b/stac_fastapi/pgstac/db.py @@ -13,11 +13,10 @@ Union, ) -import attr import orjson -from asyncpg import Connection, exceptions +from asyncpg import Connection, Pool, exceptions from buildpg import V, asyncpg, render -from fastapi import FastAPI, Request +from fastapi import FastAPI, HTTPException, Request from stac_fastapi.types.errors import ( ConflictError, DatabaseError, @@ -47,33 +46,46 @@ async def con_init(conn): ConnectionGetter = Callable[[Request, Literal["r", "w"]], AsyncIterator[Connection]] +async def _create_pool(settings: PostgresSettings) -> Pool: + """Create a connection pool.""" + return await asyncpg.create_pool( + settings.connection_string, + min_size=settings.db_min_conn_size, + max_size=settings.db_max_conn_size, + max_queries=settings.db_max_queries, + max_inactive_connection_lifetime=settings.db_max_inactive_conn_lifetime, + init=con_init, + server_settings=settings.server_settings.model_dump(), + ) + + async def connect_to_db( app: FastAPI, get_conn: Optional[ConnectionGetter] = None, postgres_settings: Optional[PostgresSettings] = None, + add_write_connection_pool: bool = False, + write_postgres_settings: Optional[PostgresSettings] = None, ) -> None: """Create connection pools & connection retriever on application.""" - app_settings = app.state.settings - if not postgres_settings: postgres_settings = PostgresSettings() - if app_settings.testing: - readpool = writepool = postgres_settings.testing_connection_string - else: - readpool = postgres_settings.reader_connection_string - writepool = postgres_settings.writer_connection_string + app.state.readpool = await _create_pool(postgres_settings) + + if add_write_connection_pool: + if not write_postgres_settings: + write_postgres_settings = PostgresSettings() + + app.state.writepool = await _create_pool(write_postgres_settings) - db = DB() - app.state.readpool = await db.create_pool(readpool, postgres_settings) - app.state.writepool = await db.create_pool(writepool, postgres_settings) app.state.get_connection = get_conn if get_conn else get_connection async def close_db_connection(app: FastAPI) -> None: """Close connection.""" await app.state.readpool.close() - await app.state.writepool.close() + if pool := getattr(app.state, "writepool", None): + await pool.close() @asynccontextmanager @@ -82,7 +94,15 @@ async def get_connection( readwrite: Literal["r", "w"] = "r", ) -> AsyncIterator[Connection]: """Retrieve connection from database conection pool.""" - pool = request.app.state.writepool if readwrite == "w" else request.app.state.readpool + pool = request.app.state.readpool + if readwrite == "w": + pool = getattr(request.app.state, "writepool", None) + if not pool: + raise HTTPException( + status_code=500, + detail="Could not find connection pool for write operations", + ) + with translate_pgstac_errors(): async with pool.acquire() as conn: yield conn @@ -131,25 +151,3 @@ def translate_pgstac_errors() -> Generator[None, None, None]: raise DatabaseError from e except exceptions.ForeignKeyViolationError as e: raise ForeignKeyError from e - - -@attr.s -class DB: - """DB class that can be used with context manager.""" - - connection_string = attr.ib(default=None) - _pool = attr.ib(default=None) - _connection = attr.ib(default=None) - - async def create_pool(self, connection_string: str, settings): - """Create a connection pool.""" - pool = await asyncpg.create_pool( - connection_string, - min_size=settings.db_min_conn_size, - max_size=settings.db_max_conn_size, - max_queries=settings.db_max_queries, - max_inactive_connection_lifetime=settings.db_max_inactive_conn_lifetime, - init=con_init, - server_settings=settings.server_settings.model_dump(), - ) - return pool diff --git a/stac_fastapi/pgstac/extensions/filter.py b/stac_fastapi/pgstac/extensions/filter.py index b6ed99ce..6b0e1949 100644 --- a/stac_fastapi/pgstac/extensions/filter.py +++ b/stac_fastapi/pgstac/extensions/filter.py @@ -25,9 +25,7 @@ async def get_queryables( under OGC CQL but it is allowed by the STAC API Filter Extension https://github.com/radiantearth/stac-api-spec/tree/master/fragments/filter#queryables """ - pool = request.app.state.readpool - - async with pool.acquire() as conn: + async with request.app.state.get_connection(request, "r") as conn: q, p = render( """ SELECT * FROM get_queryables(:collection::text); diff --git a/tests/api/test_api.py b/tests/api/test_api.py index 9185a79d..c29da8e5 100644 --- a/tests/api/test_api.py +++ b/tests/api/test_api.py @@ -742,8 +742,7 @@ async def get_collection( postgres_settings = PostgresSettings( postgres_user=database.user, postgres_pass=database.password, - postgres_host_reader=database.host, - postgres_host_writer=database.host, + postgres_host=database.host, postgres_port=database.port, postgres_dbname=database.dbname, ) @@ -770,7 +769,12 @@ async def get_collection( collections_get_request_model=collection_search_extension.GET, ) app = api.app - await connect_to_db(app, postgres_settings=postgres_settings) + await connect_to_db( + app, + postgres_settings=postgres_settings, + add_write_connection_pool=True, + write_postgres_settings=postgres_settings, + ) try: async with AsyncClient(transport=ASGITransport(app=app)) as client: response = await client.post( @@ -812,8 +816,7 @@ async def test_no_extension( postgres_settings = PostgresSettings( postgres_user=database.user, postgres_pass=database.password, - postgres_host_reader=database.host, - postgres_host_writer=database.host, + postgres_host=database.host, postgres_port=database.port, postgres_dbname=database.dbname, ) @@ -826,7 +829,12 @@ async def test_no_extension( search_post_request_model=post_request_model, ) app = api.app - await connect_to_db(app, postgres_settings=postgres_settings) + await connect_to_db( + app, + postgres_settings=postgres_settings, + add_write_connection_pool=True, + write_postgres_settings=postgres_settings, + ) try: async with AsyncClient(transport=ASGITransport(app=app)) as client: landing = await client.get("http://test/") diff --git a/tests/clients/test_postgres.py b/tests/clients/test_postgres.py index b3501bcb..68c95dbf 100644 --- a/tests/clients/test_postgres.py +++ b/tests/clients/test_postgres.py @@ -529,8 +529,7 @@ async def test_db_setup_works_with_env_vars(api_client, database, monkeypatch): """Test that the application starts successfully if the POSTGRES_* environment variables are set""" monkeypatch.setenv("POSTGRES_USER", database.user) monkeypatch.setenv("POSTGRES_PASS", database.password) - monkeypatch.setenv("POSTGRES_HOST_READER", database.host) - monkeypatch.setenv("POSTGRES_HOST_WRITER", database.host) + monkeypatch.setenv("POSTGRES_HOST", database.host) monkeypatch.setenv("POSTGRES_PORT", str(database.port)) monkeypatch.setenv("POSTGRES_DBNAME", database.dbname) @@ -567,8 +566,7 @@ async def app(self, api_client, database): postgres_settings = PostgresSettings( postgres_user=database.user, postgres_pass=database.password, - postgres_host_reader=database.host, - postgres_host_writer=database.host, + postgres_host=database.host, postgres_port=database.port, postgres_dbname=database.dbname, ) diff --git a/tests/conftest.py b/tests/conftest.py index 397ecec4..123a775b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -202,15 +202,19 @@ async def app(api_client, database): postgres_settings = PostgresSettings( postgres_user=database.user, postgres_pass=database.password, - postgres_host_reader=database.host, - postgres_host_writer=database.host, + postgres_host=database.host, postgres_port=database.port, postgres_dbname=database.dbname, ) logger.info("Creating app Fixture") time.time() app = api_client.app - await connect_to_db(app, postgres_settings=postgres_settings) + await connect_to_db( + app, + postgres_settings=postgres_settings, + add_write_connection_pool=True, + write_postgres_settings=postgres_settings, + ) yield app @@ -306,14 +310,18 @@ async def app_no_ext(database): postgres_settings = PostgresSettings( postgres_user=database.user, postgres_pass=database.password, - postgres_host_reader=database.host, - postgres_host_writer=database.host, + postgres_host=database.host, postgres_port=database.port, postgres_dbname=database.dbname, ) logger.info("Creating app Fixture") time.time() - await connect_to_db(api_client_no_ext.app, postgres_settings=postgres_settings) + await connect_to_db( + api_client_no_ext.app, + postgres_settings=postgres_settings, + add_write_connection_pool=True, + write_postgres_settings=postgres_settings, + ) yield api_client_no_ext.app await close_db_connection(api_client_no_ext.app) @@ -343,14 +351,17 @@ async def app_no_transaction(database): postgres_settings = PostgresSettings( postgres_user=database.user, postgres_pass=database.password, - postgres_host_reader=database.host, - postgres_host_writer=database.host, + postgres_host=database.host, postgres_port=database.port, postgres_dbname=database.dbname, ) logger.info("Creating app Fixture") time.time() - await connect_to_db(api.app, postgres_settings=postgres_settings) + await connect_to_db( + api.app, + postgres_settings=postgres_settings, + add_write_connection_pool=False, + ) yield api.app await close_db_connection(api.app) @@ -371,8 +382,7 @@ async def default_app(database, monkeypatch): """Test default stac-fastapi-pgstac application.""" monkeypatch.setenv("POSTGRES_USER", database.user) monkeypatch.setenv("POSTGRES_PASS", database.password) - monkeypatch.setenv("POSTGRES_HOST_READER", database.host) - monkeypatch.setenv("POSTGRES_HOST_WRITER", database.host) + monkeypatch.setenv("POSTGRES_HOST", database.host) monkeypatch.setenv("POSTGRES_PORT", str(database.port)) monkeypatch.setenv("POSTGRES_DBNAME", database.dbname) monkeypatch.delenv("ENABLED_EXTENSIONS", raising=False) @@ -383,7 +393,7 @@ async def default_app(database, monkeypatch): from stac_fastapi.pgstac.app import app - await connect_to_db(app) + await connect_to_db(app, add_write_connection_pool=True) yield app await close_db_connection(app) diff --git a/tests/resources/test_mgmt.py b/tests/resources/test_mgmt.py index 147966af..011d00aa 100644 --- a/tests/resources/test_mgmt.py +++ b/tests/resources/test_mgmt.py @@ -60,8 +60,7 @@ async def test_health_503(database): postgres_settings = PostgresSettings( postgres_user=database.user, postgres_pass=database.password, - postgres_host_reader=database.host, - postgres_host_writer=database.host, + postgres_host=database.host, postgres_port=database.port, postgres_dbname=database.dbname, ) From 3535fea3535a3d7841bf6ed99e843bfb90a406ba Mon Sep 17 00:00:00 2001 From: vincentsarago Date: Wed, 14 May 2025 17:14:47 +0200 Subject: [PATCH 2/8] rename settings from POSTGRES_* to PG* --- .github/workflows/cicd.yaml | 7 +--- CHANGES.md | 29 +++++++++++++- docker-compose.yml | 10 ++--- setup.py | 1 + stac_fastapi/pgstac/config.py | 72 ++++++++++++++++++++++++++++++---- stac_fastapi/pgstac/db.py | 2 +- tests/api/test_api.py | 22 +++++------ tests/clients/test_postgres.py | 20 +++++----- tests/conftest.py | 42 ++++++++++---------- tests/resources/test_mgmt.py | 10 ++--- tests/test_config.py | 60 ++++++++++++++++++++++++++++ 11 files changed, 206 insertions(+), 69 deletions(-) create mode 100644 tests/test_config.py diff --git a/.github/workflows/cicd.yaml b/.github/workflows/cicd.yaml index 682e2c31..50ce577a 100644 --- a/.github/workflows/cicd.yaml +++ b/.github/workflows/cicd.yaml @@ -90,13 +90,10 @@ jobs: - name: Load data and validate run: python -m stac_fastapi.pgstac.app & ./scripts/wait-for-it.sh localhost:8080 && python ./scripts/ingest_joplin.py http://localhost:8080 && ./scripts/validate http://localhost:8080 env: - POSTGRES_USER: username - POSTGRES_PASS: password - POSTGRES_DBNAME: postgis - POSTGRES_HOST: localhost - POSTGRES_PORT: 5432 PGUSER: username PGPASSWORD: password + PGHOST: localhost + PGPORT: 5432 PGDATABASE: postgis APP_HOST: 0.0.0.0 APP_PORT: 8080 diff --git a/CHANGES.md b/CHANGES.md index ca03ba9c..73e0cb10 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,7 +4,34 @@ ### Changed -- rename `POSTGRES_HOST_READER` to `POSTGRES_HOST` in config **breaking change** +- rename `POSTGRES_HOST_READER` to `PGHOST` in config **breaking change** +- rename `POSTGRES_USER` to `PGUSER` in config **breaking change** +- rename `POSTGRES_PASS` to `PGPASSWORD` in config **breaking change** +- rename `POSTGRES_PORT` to `PGPORT` in config **breaking change** +- rename `POSTGRES_DBNAME` to `PGDBNAME` in config **breaking change** + ```python + from stac_fastapi.pgstac.config import PostgresSettings + + # before + settings = PostgresSettings( + postgres_user="user", + postgres_pass="password", + postgres_host_reader="0.0.0.0", + postgres_host_writer="0.0.0.0", + postgres_port=1111, + postgres_dbname="pgstac", + ) + + # now + settings = PostgresSettings( + pguser="user", + pgpassword="password", + pghost="0.0.0.0", + pgport=1111, + pgdatabase="pgstac", + ) + ``` + - rename `reader_connection_string` to `connection_string` in `PostgresSettings` class **breaking change** - add `ENABLE_TRANSACTIONS_EXTENSIONS` env variable to enable `transaction` extensions - disable transaction and bulk_transactions extensions by default **breaking change** diff --git a/docker-compose.yml b/docker-compose.yml index b0ce3462..2a3dfaea 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -8,11 +8,11 @@ services: - APP_PORT=8082 - RELOAD=true - ENVIRONMENT=local - - POSTGRES_USER=username - - POSTGRES_PASS=password - - POSTGRES_DBNAME=postgis - - POSTGRES_HOST=database - - POSTGRES_PORT=5432 + - PGUSER=username + - PGPASS=password + - PGDATABASE=postgis + - PGHOST=database + - PGPORT=5432 - WEB_CONCURRENCY=10 - VSI_CACHE=TRUE - GDAL_HTTP_MERGE_CONSECUTIVE_RANGES=YES diff --git a/setup.py b/setup.py index a9da53d1..90436eaf 100644 --- a/setup.py +++ b/setup.py @@ -17,6 +17,7 @@ "brotli_asgi", "pygeofilter>=0.2", "pypgstac>=0.8,<0.10", + "typing_extensions>=4.9.0", ] extra_reqs = { diff --git a/stac_fastapi/pgstac/config.py b/stac_fastapi/pgstac/config.py index ea21709d..18fc0358 100644 --- a/stac_fastapi/pgstac/config.py +++ b/stac_fastapi/pgstac/config.py @@ -1,9 +1,10 @@ """Postgres API configuration.""" -from typing import List, Type +import warnings +from typing import Annotated, Any, List, Optional, Type from urllib.parse import quote_plus as quote -from pydantic import BaseModel, field_validator +from pydantic import BaseModel, Field, field_validator, model_validator from pydantic_settings import BaseSettings, SettingsConfigDict from stac_fastapi.types.config import ApiSettings @@ -61,11 +62,44 @@ class PostgresSettings(BaseSettings): invalid_id_chars: list of characters that are not allowed in item or collection ids. """ - postgres_user: str - postgres_pass: str - postgres_host: str - postgres_port: int - postgres_dbname: str + postgres_user: Annotated[ + Optional[str], + Field( + deprecated="`postgres_user` is deprecated, please use `pguser`", default=None + ), + ] + postgres_pass: Annotated[ + Optional[str], + Field( + deprecated="`postgres_pass` is deprecated, please use `pgpassword`", + default=None, + ), + ] + postgres_host: Annotated[ + Optional[str], + Field( + deprecated="`postgres_host` is deprecated, please use `pghost`", default=None + ), + ] + postgres_port: Annotated[ + Optional[int], + Field( + deprecated="`postgres_port` is deprecated, please use `pgport`", default=None + ), + ] + postgres_dbname: Annotated[ + Optional[str], + Field( + deprecated="`postgres_dbname` is deprecated, please use `pgdatabase`", + default=None, + ), + ] + + pguser: str + pgpassword: str + pghost: str + pgport: int + pgdatabase: str db_min_conn_size: int = 1 db_max_conn_size: int = 10 @@ -76,10 +110,32 @@ class PostgresSettings(BaseSettings): model_config = {"env_file": ".env", "extra": "ignore"} + @model_validator(mode="before") + @classmethod + def _pg_settings_compat(cls, data: Any) -> Any: + if isinstance(data, dict): + compat = { + "postgres_user": "pguser", + "postgres_pass": "pgpassword", + "postgres_host": "pghost", + "postgres_port": "pgport", + "postgres_dbname": "pgdatabase", + } + for old_key, new_key in compat.items(): + if val := data.get(old_key, None): + warnings.warn( + f"`{old_key}` is deprecated, please use `{new_key}`", + DeprecationWarning, + stacklevel=1, + ) + data[new_key] = val + + return data + @property def connection_string(self): """Create reader psql connection string.""" - return f"postgresql://{self.postgres_user}:{quote(self.postgres_pass)}@{self.postgres_host}:{self.postgres_port}/{self.postgres_dbname}" + return f"postgresql://{self.pguser}:{quote(self.pgpassword)}@{self.pghost}:{self.pgport}/{self.pgdatabase}" class Settings(ApiSettings): diff --git a/stac_fastapi/pgstac/db.py b/stac_fastapi/pgstac/db.py index 4aa48bd2..302d43f3 100644 --- a/stac_fastapi/pgstac/db.py +++ b/stac_fastapi/pgstac/db.py @@ -74,7 +74,7 @@ async def connect_to_db( if add_write_connection_pool: if not write_postgres_settings: - write_postgres_settings = PostgresSettings() + write_postgres_settings = postgres_settings app.state.writepool = await _create_pool(write_postgres_settings) diff --git a/tests/api/test_api.py b/tests/api/test_api.py index c29da8e5..bd2a3641 100644 --- a/tests/api/test_api.py +++ b/tests/api/test_api.py @@ -740,11 +740,11 @@ async def get_collection( ) postgres_settings = PostgresSettings( - postgres_user=database.user, - postgres_pass=database.password, - postgres_host=database.host, - postgres_port=database.port, - postgres_dbname=database.dbname, + pguser=database.user, + pgpassword=database.password, + pghost=database.host, + pgport=database.port, + pgdatabase=database.dbname, ) extensions = [ @@ -773,7 +773,6 @@ async def get_collection( app, postgres_settings=postgres_settings, add_write_connection_pool=True, - write_postgres_settings=postgres_settings, ) try: async with AsyncClient(transport=ASGITransport(app=app)) as client: @@ -814,11 +813,11 @@ async def test_no_extension( enable_response_models=validation, ) postgres_settings = PostgresSettings( - postgres_user=database.user, - postgres_pass=database.password, - postgres_host=database.host, - postgres_port=database.port, - postgres_dbname=database.dbname, + pguser=database.user, + pgpassword=database.password, + pghost=database.host, + pgport=database.port, + pgdatabase=database.dbname, ) extensions = [] post_request_model = create_post_request_model(extensions, base_model=PgstacSearch) @@ -833,7 +832,6 @@ async def test_no_extension( app, postgres_settings=postgres_settings, add_write_connection_pool=True, - write_postgres_settings=postgres_settings, ) try: async with AsyncClient(transport=ASGITransport(app=app)) as client: diff --git a/tests/clients/test_postgres.py b/tests/clients/test_postgres.py index 68c95dbf..4e066945 100644 --- a/tests/clients/test_postgres.py +++ b/tests/clients/test_postgres.py @@ -527,11 +527,11 @@ async def test_create_bulk_items_id_mismatch( async def test_db_setup_works_with_env_vars(api_client, database, monkeypatch): """Test that the application starts successfully if the POSTGRES_* environment variables are set""" - monkeypatch.setenv("POSTGRES_USER", database.user) - monkeypatch.setenv("POSTGRES_PASS", database.password) - monkeypatch.setenv("POSTGRES_HOST", database.host) - monkeypatch.setenv("POSTGRES_PORT", str(database.port)) - monkeypatch.setenv("POSTGRES_DBNAME", database.dbname) + monkeypatch.setenv("PGUSER", database.user) + monkeypatch.setenv("PGPASSWORD", database.password) + monkeypatch.setenv("PGHOST", database.host) + monkeypatch.setenv("PGPORT", str(database.port)) + monkeypatch.setenv("PGDATABASE", database.dbname) await connect_to_db(api_client.app) await close_db_connection(api_client.app) @@ -564,11 +564,11 @@ async def app(self, api_client, database): app fixture override to setup app with a customized db connection getter """ postgres_settings = PostgresSettings( - postgres_user=database.user, - postgres_pass=database.password, - postgres_host=database.host, - postgres_port=database.port, - postgres_dbname=database.dbname, + pguser=database.user, + pgpassword=database.password, + pghost=database.host, + pgport=database.port, + pgdatabase=database.dbname, ) logger.debug("Customizing app setup") diff --git a/tests/conftest.py b/tests/conftest.py index 123a775b..05846bec 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -200,11 +200,11 @@ def api_client(request): @pytest.fixture(scope="function") async def app(api_client, database): postgres_settings = PostgresSettings( - postgres_user=database.user, - postgres_pass=database.password, - postgres_host=database.host, - postgres_port=database.port, - postgres_dbname=database.dbname, + pguser=database.user, + pgpassword=database.password, + pghost=database.host, + pgport=database.port, + pgdatabase=database.dbname, ) logger.info("Creating app Fixture") time.time() @@ -213,7 +213,6 @@ async def app(api_client, database): app, postgres_settings=postgres_settings, add_write_connection_pool=True, - write_postgres_settings=postgres_settings, ) yield app @@ -308,11 +307,11 @@ async def app_no_ext(database): ) postgres_settings = PostgresSettings( - postgres_user=database.user, - postgres_pass=database.password, - postgres_host=database.host, - postgres_port=database.port, - postgres_dbname=database.dbname, + pguser=database.user, + pgpassword=database.password, + pghost=database.host, + pgport=database.port, + pgdatabase=database.dbname, ) logger.info("Creating app Fixture") time.time() @@ -320,7 +319,6 @@ async def app_no_ext(database): api_client_no_ext.app, postgres_settings=postgres_settings, add_write_connection_pool=True, - write_postgres_settings=postgres_settings, ) yield api_client_no_ext.app await close_db_connection(api_client_no_ext.app) @@ -349,11 +347,11 @@ async def app_no_transaction(database): ) postgres_settings = PostgresSettings( - postgres_user=database.user, - postgres_pass=database.password, - postgres_host=database.host, - postgres_port=database.port, - postgres_dbname=database.dbname, + pguser=database.user, + pgpassword=database.password, + pghost=database.host, + pgport=database.port, + pgdatabase=database.dbname, ) logger.info("Creating app Fixture") time.time() @@ -380,11 +378,11 @@ async def app_client_no_transaction(app_no_transaction): @pytest.fixture(scope="function") async def default_app(database, monkeypatch): """Test default stac-fastapi-pgstac application.""" - monkeypatch.setenv("POSTGRES_USER", database.user) - monkeypatch.setenv("POSTGRES_PASS", database.password) - monkeypatch.setenv("POSTGRES_HOST", database.host) - monkeypatch.setenv("POSTGRES_PORT", str(database.port)) - monkeypatch.setenv("POSTGRES_DBNAME", database.dbname) + monkeypatch.setenv("PGUSER", database.user) + monkeypatch.setenv("PGPASSWORD", database.password) + monkeypatch.setenv("PGHOST", database.host) + monkeypatch.setenv("PGPORT", str(database.port)) + monkeypatch.setenv("PGDATABASE", database.dbname) monkeypatch.delenv("ENABLED_EXTENSIONS", raising=False) monkeypatch.setenv("ENABLE_TRANSACTIONS_EXTENSIONS", "TRUE") diff --git a/tests/resources/test_mgmt.py b/tests/resources/test_mgmt.py index 011d00aa..e2bd9c05 100644 --- a/tests/resources/test_mgmt.py +++ b/tests/resources/test_mgmt.py @@ -58,11 +58,11 @@ async def test_health_503(database): # No lifespan so no `get_connection` is application state postgres_settings = PostgresSettings( - postgres_user=database.user, - postgres_pass=database.password, - postgres_host=database.host, - postgres_port=database.port, - postgres_dbname=database.dbname, + pguser=database.user, + pgpassword=database.password, + pghost=database.host, + pgport=database.port, + pgdatabase=database.dbname, ) # Create connection pool but close it just after await connect_to_db(api.app, postgres_settings=postgres_settings) diff --git a/tests/test_config.py b/tests/test_config.py new file mode 100644 index 00000000..4300c752 --- /dev/null +++ b/tests/test_config.py @@ -0,0 +1,60 @@ +"""test config.""" + +import warnings + +import pytest + +from stac_fastapi.pgstac.config import PostgresSettings + + +async def test_pg_settings_with_env(monkeypatch): + """Test PostgresSettings with PG* environment variables""" + monkeypatch.setenv("PGUSER", "username") + monkeypatch.setenv("PGPASSWORD", "password") + monkeypatch.setenv("PGHOST", "0.0.0.0") + monkeypatch.setenv("PGPORT", "1111") + monkeypatch.setenv("PGDATABASE", "pgstac") + assert PostgresSettings(_env_file=None) + + +async def test_pg_settings_with_env_postgres(monkeypatch): + """Test PostgresSettings with POSTGRES_* environment variables""" + monkeypatch.setenv("POSTGRES_USER", "username") + monkeypatch.setenv("POSTGRES_PASS", "password") + monkeypatch.setenv("POSTGRES_HOST", "0.0.0.0") + monkeypatch.setenv("POSTGRES_PORT", "1111") + monkeypatch.setenv("POSTGRES_DBNAME", "pgstac") + assert PostgresSettings(_env_file=None) + + +async def test_pg_settings_attributes(monkeypatch): + """Test PostgresSettings with attributes""" + with warnings.catch_warnings(): + warnings.simplefilter("error") + settings = PostgresSettings( + pguser="user", + pgpassword="password", + pghost="0.0.0.0", + pgport=1111, + pgdatabase="pgstac", + _env_file=None, + ) + assert settings.pghost == "0.0.0.0" + + # Compat, should work with old style postgres_ attributes + # Should raise warnings on set attribute + with pytest.warns(DeprecationWarning) as record: + settings = PostgresSettings( + postgres_user="user", + postgres_pass="password", + postgres_host="0.0.0.0", + postgres_port=1111, + postgres_dbname="pgstac", + _env_file=None, + ) + assert settings.pghost == "0.0.0.0" + assert len(record) == 5 + + # Should raise warning when accessing deprecated attributes + with pytest.warns(DeprecationWarning): + assert settings.postgres_host == "0.0.0.0" From 00c23ca1167c92ecb79f08f64b69e7c9fd4a5796 Mon Sep 17 00:00:00 2001 From: vincentsarago Date: Wed, 14 May 2025 17:17:18 +0200 Subject: [PATCH 3/8] catch warnings --- tests/test_config.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_config.py b/tests/test_config.py index 4300c752..4bf96bed 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -24,7 +24,9 @@ async def test_pg_settings_with_env_postgres(monkeypatch): monkeypatch.setenv("POSTGRES_HOST", "0.0.0.0") monkeypatch.setenv("POSTGRES_PORT", "1111") monkeypatch.setenv("POSTGRES_DBNAME", "pgstac") - assert PostgresSettings(_env_file=None) + with pytest.warns(DeprecationWarning) as record: + assert PostgresSettings(_env_file=None) + assert len(record) == 5 async def test_pg_settings_attributes(monkeypatch): From d263d72c19c45791ab74954c654fcc97617d1c53 Mon Sep 17 00:00:00 2001 From: vincentsarago Date: Wed, 14 May 2025 17:27:59 +0200 Subject: [PATCH 4/8] typo --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 73e0cb10..b1591a44 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,7 +8,7 @@ - rename `POSTGRES_USER` to `PGUSER` in config **breaking change** - rename `POSTGRES_PASS` to `PGPASSWORD` in config **breaking change** - rename `POSTGRES_PORT` to `PGPORT` in config **breaking change** -- rename `POSTGRES_DBNAME` to `PGDBNAME` in config **breaking change** +- rename `POSTGRES_DBNAME` to `PGDATABASE` in config **breaking change** ```python from stac_fastapi.pgstac.config import PostgresSettings From d85d10a4a4888129d4a2c23b012a1a9b8a8b4850 Mon Sep 17 00:00:00 2001 From: Vincent Sarago Date: Thu, 15 May 2025 21:20:02 +0200 Subject: [PATCH 5/8] Update stac_fastapi/pgstac/config.py Co-authored-by: Henry Rodman --- stac_fastapi/pgstac/config.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/stac_fastapi/pgstac/config.py b/stac_fastapi/pgstac/config.py index 18fc0358..29d920f4 100644 --- a/stac_fastapi/pgstac/config.py +++ b/stac_fastapi/pgstac/config.py @@ -75,10 +75,16 @@ class PostgresSettings(BaseSettings): default=None, ), ] - postgres_host: Annotated[ + postgres_host_reader: Annotated[ Optional[str], Field( - deprecated="`postgres_host` is deprecated, please use `pghost`", default=None + deprecated="`postgres_host_reader` is deprecated, please use `pghost`", default=None + ), + ] + postgres_host_writer: Annotated[ + Optional[str], + Field( + deprecated="`postgres_host_writer` is deprecated, please use `pghost`", default=None ), ] postgres_port: Annotated[ From 060bd0028a3fb9413c7ec77c0f4617fe1e951230 Mon Sep 17 00:00:00 2001 From: vincentsarago Date: Thu, 15 May 2025 21:27:53 +0200 Subject: [PATCH 6/8] update from review --- stac_fastapi/pgstac/config.py | 18 +++++++++++++++--- tests/test_config.py | 22 ++++++++++++++++++---- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/stac_fastapi/pgstac/config.py b/stac_fastapi/pgstac/config.py index 29d920f4..f799201c 100644 --- a/stac_fastapi/pgstac/config.py +++ b/stac_fastapi/pgstac/config.py @@ -78,13 +78,15 @@ class PostgresSettings(BaseSettings): postgres_host_reader: Annotated[ Optional[str], Field( - deprecated="`postgres_host_reader` is deprecated, please use `pghost`", default=None + deprecated="`postgres_host_reader` is deprecated, please use `pghost`", + default=None, ), ] postgres_host_writer: Annotated[ Optional[str], Field( - deprecated="`postgres_host_writer` is deprecated, please use `pghost`", default=None + deprecated="`postgres_host_writer` is deprecated, please use `pghost`", + default=None, ), ] postgres_port: Annotated[ @@ -123,7 +125,8 @@ def _pg_settings_compat(cls, data: Any) -> Any: compat = { "postgres_user": "pguser", "postgres_pass": "pgpassword", - "postgres_host": "pghost", + "postgres_host_reader": "pghost", + "postgres_host_writer": "pghost", "postgres_port": "pgport", "postgres_dbname": "pgdatabase", } @@ -136,6 +139,15 @@ def _pg_settings_compat(cls, data: Any) -> Any: ) data[new_key] = val + if (pgh_reader := data.get("postgres_host_reader")) and ( + pgh_writer := data.get("postgres_host_writer") + ): + if pgh_reader != pgh_writer: + raise ValueError( + "In order to use different host values for reading and writing " + "you must explicitly provide write_postgres_settings to the connect_to_db function" + ) + return data @property diff --git a/tests/test_config.py b/tests/test_config.py index 4bf96bed..e05eb5ef 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -3,6 +3,7 @@ import warnings import pytest +from pydantic import ValidationError from stac_fastapi.pgstac.config import PostgresSettings @@ -21,12 +22,13 @@ async def test_pg_settings_with_env_postgres(monkeypatch): """Test PostgresSettings with POSTGRES_* environment variables""" monkeypatch.setenv("POSTGRES_USER", "username") monkeypatch.setenv("POSTGRES_PASS", "password") - monkeypatch.setenv("POSTGRES_HOST", "0.0.0.0") + monkeypatch.setenv("POSTGRES_HOST_READER", "0.0.0.0") + monkeypatch.setenv("POSTGRES_HOST_WRITER", "0.0.0.0") monkeypatch.setenv("POSTGRES_PORT", "1111") monkeypatch.setenv("POSTGRES_DBNAME", "pgstac") with pytest.warns(DeprecationWarning) as record: assert PostgresSettings(_env_file=None) - assert len(record) == 5 + assert len(record) == 6 async def test_pg_settings_attributes(monkeypatch): @@ -49,7 +51,7 @@ async def test_pg_settings_attributes(monkeypatch): settings = PostgresSettings( postgres_user="user", postgres_pass="password", - postgres_host="0.0.0.0", + postgres_host_reader="0.0.0.0", postgres_port=1111, postgres_dbname="pgstac", _env_file=None, @@ -59,4 +61,16 @@ async def test_pg_settings_attributes(monkeypatch): # Should raise warning when accessing deprecated attributes with pytest.warns(DeprecationWarning): - assert settings.postgres_host == "0.0.0.0" + assert settings.postgres_host_reader == "0.0.0.0" + + with pytest.raises(ValidationError): + with pytest.warns(DeprecationWarning) as record: + PostgresSettings( + postgres_user="user", + postgres_pass="password", + postgres_host_reader="0.0.0.0", + postgres_host_writer="1.1.1.1", + postgres_port=1111, + postgres_dbname="pgstac", + _env_file=None, + ) From 5ac7a2969a2b23c26d31f050193643de5d35e039 Mon Sep 17 00:00:00 2001 From: vincentsarago Date: Fri, 16 May 2025 07:51:23 +0200 Subject: [PATCH 7/8] update docstring --- stac_fastapi/pgstac/config.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/stac_fastapi/pgstac/config.py b/stac_fastapi/pgstac/config.py index f799201c..76b2c06a 100644 --- a/stac_fastapi/pgstac/config.py +++ b/stac_fastapi/pgstac/config.py @@ -53,11 +53,11 @@ class PostgresSettings(BaseSettings): """Postgres-specific API settings. Attributes: - postgres_user: postgres username. - postgres_pass: postgres password. - postgres_host: hostname for the connection. - postgres_port: database port. - postgres_dbname: database name. + pguser: postgres username. + pgpassword: postgres password. + pghost: hostname for the connection. + pgport: database port. + pgdatabase: database name. use_api_hydrate: perform hydration of stac items within stac-fastapi. invalid_id_chars: list of characters that are not allowed in item or collection ids. """ From 240988fb53dc2400d3f6082a71de059f1faf7fde Mon Sep 17 00:00:00 2001 From: vincentsarago Date: Fri, 16 May 2025 10:04:53 +0200 Subject: [PATCH 8/8] add docs --- CHANGES.md | 2 +- docs/api/stac_fastapi/pgstac/version.md | 1 - docs/settings.md | 64 +++++++++++++++++++++++++ mkdocs.yml | 1 + stac_fastapi/pgstac/config.py | 11 +++-- 5 files changed, 74 insertions(+), 5 deletions(-) delete mode 100644 docs/api/stac_fastapi/pgstac/version.md create mode 100644 docs/settings.md diff --git a/CHANGES.md b/CHANGES.md index 8257182d..426768a1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -224,7 +224,7 @@ As a part of this release, this repository was extracted from the main ### Added - Nginx service as second docker-compose stack to demonstrate proxy ([#503](https://github.com/stac-utils/stac-fastapi/pull/503)) -- Validation checks in CI using [stac-api-validator](github.com/stac-utils/stac-api-validator) ([#508](https://github.com/stac-utils/stac-fastapi/pull/508)) +- Validation checks in CI using [stac-api-validator](https://github.com/stac-utils/stac-api-validator) ([#508](https://github.com/stac-utils/stac-fastapi/pull/508)) - Required links to the sqlalchemy ItemCollection endpoint ([#508](https://github.com/stac-utils/stac-fastapi/pull/508)) - Publication of docker images to GHCR ([#525](https://github.com/stac-utils/stac-fastapi/pull/525)) diff --git a/docs/api/stac_fastapi/pgstac/version.md b/docs/api/stac_fastapi/pgstac/version.md deleted file mode 100644 index 2712843c..00000000 --- a/docs/api/stac_fastapi/pgstac/version.md +++ /dev/null @@ -1 +0,0 @@ -::: stac_fastapi.pgstac.version diff --git a/docs/settings.md b/docs/settings.md new file mode 100644 index 00000000..9360622a --- /dev/null +++ b/docs/settings.md @@ -0,0 +1,64 @@ + + +### Application Extension + +The default `stac-fastapi-pgstac` application comes will **all** extensions enabled (except transaction). Users can use `ENABLED_EXTENSIONS` environment variable to limit the supported extensions. + +Available values for `ENABLED_EXTENSIONS`: + +- `query` +- `sort` +- `fields` +- `filter` +- `free_text` (only for collection-search) +- `pagination` +- `collection_search` + +Example: `ENABLED_EXTENSIONS="pagination,sort"` + + +Since `6.0.0`, the transaction extension is not enabled by default. To add the transaction endpoints, users can set `ENABLE_TRANSACTIONS_EXTENSIONS=TRUE/YES/1`. + +### Database config + +- `PGUSER`: postgres username +- `PGPASSWORD`: postgres password +- `PGHOST`: hostname for the connection +- `PGPORT`: database port +- `PGDATABASE`: database name +- `DB_MIN_CONN_SIZE`: Number of connection the pool will be initialized with. Defaults to `1` +- `DB_MAX_CONN_SIZE` Max number of connections in the pool. Defaults to `10` +- `DB_MAX_QUERIES`: Number of queries after a connection is closed and replaced with a new connection. Defaults to `50000` +- `DB_MAX_INACTIVE_CONN_LIFETIME`: Number of seconds after which inactive connections in the pool will be closed. Defaults to `300` +- `SEARCH_PATH`: Postgres search path. Defaults to `"pgstac,public"` +- `APPLICATION_NAME`: PgSTAC Application name. Defaults to `"pgstac"` + +##### Deprecated + +In version `6.0.0` we've renamed the PG configuration variable to match the official naming convention: + +- `POSTGRES_USER` -> `PGUSER` +- `POSTGRES_PASS` -> `PGPASSWORD` +- `POSTGRES_HOST_READER` -> `PGHOST` +- `POSTGRES_HOST_WRITER` -> `PGHOST`* +- `POSTGRES_PORT` -> `PGPORT` +- `POSTGRES_DBNAME` -> `PGDATABASE` + +\* Since version `6.0`, users cannot set a different host for `writer` and `reader` database but will need to customize the application and pass a specific `stac_fastapi.pgstac.config.PostgresSettings` instance to the `connect_to_db` function. + +### Validation/Serialization + +- `ENABLE_RESPONSE_MODELS`: use pydantic models to validate endpoint responses. Defaults to `False` +- `ENABLE_DIRECT_RESPONSE`: by-pass the default FastAPI serialization by wrapping the endpoint responses into `starlette.Response` classes. Defaults to `False` + +### Misc + +- `STAC_FASTAPI_VERSION` (string) is the version number of your API instance (this is not the STAC version) +- `STAC FASTAPI_TITLE` (string) should be a self-explanatory title for your API +- `STAC FASTAPI_DESCRIPTION` (string) should be a good description for your API. It can contain CommonMark +- `STAC_FASTAPI_LANDING_ID` (string) is a unique identifier for your Landing page +- `ROOT_PATH`: set application root-path (when using proxy) +- `CORS_ORIGINS`: A list of origins that should be permitted to make cross-origin requests. Defaults to `*` +- `CORS_METHODS`: A list of HTTP methods that should be allowed for cross-origin requests. Defaults to `"GET,POST,OPTIONS"` +- `USE_API_HYDRATE`: perform hydration of stac items within stac-fastapi +- `INVALID_ID_CHARS`: list of characters that are not allowed in item or collection ids (used in Transaction endpoints) diff --git a/mkdocs.yml b/mkdocs.yml index 215c838f..886b5c3c 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -15,6 +15,7 @@ extra: # Layout nav: - Home: "index.md" + - Configuration: "settings.md" - API: - stac_fastapi.pgstac: - module: api/stac_fastapi/pgstac/index.md diff --git a/stac_fastapi/pgstac/config.py b/stac_fastapi/pgstac/config.py index 76b2c06a..c8741812 100644 --- a/stac_fastapi/pgstac/config.py +++ b/stac_fastapi/pgstac/config.py @@ -58,8 +58,7 @@ class PostgresSettings(BaseSettings): pghost: hostname for the connection. pgport: database port. pgdatabase: database name. - use_api_hydrate: perform hydration of stac items within stac-fastapi. - invalid_id_chars: list of characters that are not allowed in item or collection ids. + """ postgres_user: Annotated[ @@ -157,7 +156,13 @@ def connection_string(self): class Settings(ApiSettings): - """Api Settings.""" + """API settings. + + Attributes: + use_api_hydrate: perform hydration of stac items within stac-fastapi. + invalid_id_chars: list of characters that are not allowed in item or collection ids. + + """ use_api_hydrate: bool = False invalid_id_chars: List[str] = DEFAULT_INVALID_ID_CHARS