feat(cli): resolve PostgreSQL settings from app.yaml, not just env - #143
Merged
Conversation
added 2 commits
August 7, 2026 10:34
create_server_from_config built an explicit DatabaseConfig with mongodb's uri/name and dynamodb's table/region, but nothing for postgres. The DSN reached the driver only through jvspatial's own env read, so an app.yaml database.uri was silently ignored for postgres while working for every other backend -- the one case where a documented config path did nothing. Thread database.uri / pooler_mode / min_pool_size / max_pool_size through for postgres and postgresql. database.uri is shared with mongodb; database.type decides how it is read. Unset values stay None so PostgresDB's own defaults still apply, and a non-integer pool size is logged and ignored rather than failing startup -- a typo'd pool size should not stop a server from booting. Requires jvspatial 0.0.17. Until then DatabaseConfig discarded anything passed by field name (its aliased fields lacked populate_by_name), so this passthrough would have been written correctly and done nothing -- which is how that upstream bug was found. Pin bumped. Verified against the released 0.0.17 wheel, not a local checkout: full suite green, and scripts/smoke_postgres.sh 15/15 including recall across a server restart. Docs drop the "DSN is env-only" caveat and gain a worked app.yaml example. Also refreshes two stale pin references in jvspatial-integration.md sections 1 and 5.
The new tests build a real Server with db_type=postgres, which now instantiates PostgresDB and imports asyncpg. asyncpg is an optional extra, so CI did not have it and the job failed with ImportError -- while passing locally purely because this machine had the driver installed from earlier Postgres work. A test that depends on an undeclared extra is a test that passes for the wrong reason. Add asyncpg to the [test] extra so CI exercises the feature, and guard the module with pytest.importorskip so anyone running the suite without the extra skips instead of erroring (matching jvspatial's own postgres tests). Verified both ways: 7 passed with the driver, cleanly skipped without it. Also syncs requirements.txt and requirements-all.txt, which still pinned jvspatial==0.0.15 -- stale since the 0.0.16 bump and now two releases behind pyproject.
11 tasks
…passthrough # Conflicts: # CHANGELOG.md # requirements-all.txt # requirements.txt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
create_server_from_configthreads mongodb'suri/nameand dynamodb'stable/regioninto jvspatial'sDatabaseConfig— but had nothing for postgres. The DSN reached the driver only through jvspatial's own env read, so anapp.yamldatabase.uriwas silently ignored for postgres while working for every other backend.This wires the four
postgres_*settings through, and bumps the pin tojvspatial==0.0.17.Related issues
Depends on TrueSelph/jvspatial#37, released in 0.0.17 (now on PyPI). Completes the Postgres work from #139 / #140.
Type of change
app.yamlnow configures PostgresChanges
jvagent/cli/server_config.pydatabase.uri/pooler_mode/min_pool_size/max_pool_size→DatabaseConfigforpostgres/postgresqltests/cli/test_server_config_postgres.pypyproject.toml0.0.16→0.0.17docs/postgres.mdapp.yamlexample.env.example,environment-keys-reference.md.planning/reference/jvspatial-integration.mdCHANGELOG.md[Unreleased]entryChecklist
CONTRIBUTING.mdand the localjvagent/cli/CLAUDE.md.pre-commit run --all-filespasses.pytest tests/passes — 0 failures, run against the released0.0.17wheel.file:linein the description.CHANGELOG.mdupdated.Notes for reviewers
Why the pin has to move. Before 0.0.17,
DatabaseConfigdiscarded anything passed by field name — its aliased fields lackedpopulate_by_name, soDatabaseConfig(postgres_dsn=...)silently producedNone. This passthrough would have been written correctly and done nothing. That is in fact how the upstream bug was found: the code looked right and the tests failed. Worth knowing because the same shape was also silently breaking DynamoDB'sapp.yamlconfig, for as long as those fields have existed.Shared
database.uri. Postgres reads the sameapp.yamlkey as mongodb, withdatabase.typedeciding interpretation. The alternative was adatabase.postgres_uri, which would have made the stanza backend-specific in a way the existing keys are not. Easy to change if you'd rather they were distinct.A typo shouldn't stop the server booting. A non-integer pool size is logged and dropped rather than raised, so the driver default applies. Covered by
test_non_integer_pool_size_is_ignored.Verified against the released wheel, not a checkout.
pip install jvspatial==0.0.17from PyPI: full suite 0 failures, andscripts/smoke_postgres.sh15/15 — bootstrap, serve, JWT login, agent turn, server restart, and recall of pre-restart state from Postgres.One process note: my first verification of this branch failed, because
uv pip install -ewith a relative--pythonpath installed into the sibling repo's venv instead of this one, andimport jvspatial; print(jvspatial.__file__)reported the source path anyway due to cwd shadowing. If you verify locally, check the specific module —python -c "import jvspatial.api.config_groups as cg; print(cg.__file__)"— which is unambiguous.Steps to test
pytest tests/cli/test_server_config_postgres.py -q scripts/smoke_postgres.sh # spins up its own containerFor the new path specifically, an
app.yamlwith noJVSPATIAL_POSTGRES_*env set: