Skip to content

Python: Prefer injected Postgres clients over connection settings - #9983

Open
ZedingZhang (ZedingZhang) wants to merge 1 commit into
microsoft:mainfrom
ZedingZhang:fix/postgres-injected-client-settings-9958
Open

ZedingZhang (ZedingZhang) wants to merge 1 commit into
microsoft:mainfrom
ZedingZhang:fix/postgres-injected-client-settings-9958

Conversation

@ZedingZhang

@ZedingZhang ZedingZhang (ZedingZhang) commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Motivation & Context

PostgresStore and PostgresCollection raise ValueError when an injected Psycopg connection or pool is supplied alongside connection-string or .env options. The injected client's configuration should take precedence, allowing applications to reuse their client with shared configuration.

Description & Review Guide

  • What are the major changes? Remove the conflicting-settings rejection from the shared client-construction helper. Extend the existing regression test to 32 combinations of store/collection, connection/pool, and redundant settings. Update the invalid-client test and the parameter descriptions that documented the rejection.
  • What is the impact of these changes? Injected connections and pools can be used with redundant, empty, or invalid connection settings. The supplied schema is applied, and wrapper cleanup does not close the client or commit/roll back its transaction.
  • What do you want reviewers to focus on? Regression coverage verifies that settings loading and pool creation are skipped for injected clients, and that invalid client types are still rejected. The regression fails on the original implementation and passes with the fix.

Related Issue

Fixes #9958. The issue has the ready-for-implementation label; no competing open PR was found before submission.

AI Assistance

  • No material AI assistance was used.
  • This is an AI-assisted contribution.

AI assistance details: Codex assisted with investigation, implementation, regression tests, parameter documentation, and this PR description. Codex executed the reported checks; I reviewed all submitted content and take responsibility for it.

Contribution Checklist

  • The code builds clean without any errors or warnings — Postgres sdist and wheel built successfully.
  • All unit tests pass, and I have added new tests where possible — Postgres unit suite: 176 passed, 43 integration tests deselected. Ruff formatting/lint, strict source Pyright, all five test type checkers, and README code-block checks pass. No live PostgreSQL integration tests were run.
  • The PR follows the Contribution Guidelines
  • This PR links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues were identified for the change to prioritize caller-supplied Postgres connections and pools.

No open issues 🎉

What changed in this PR

Updates the Python Postgres connector to prefer caller-supplied connections and pools over connection-string and .env settings, addressing #9958 while preserving caller ownership.

Changes:

  • Removes the conflicting-settings rejection for injected clients.
  • Expands regression coverage for settings bypass, schema handling, and client ownership; retains invalid-client rejection coverage.
  • Updates README and constructor documentation to explain the new precedence.
File Description
python/​packages/​postgres/​tests/​test_postgres.py Updates the invalid-client case to retain rejection coverage.
python/​packages/​postgres/​tests/​test_postgres_settings.py Tests injected-client precedence and ownership across 32 combinations.
python/​packages/​postgres/​README.md Documents that injected clients override connection settings.
python/​packages/​postgres/​agent_framework_postgres/​_vector_store.py Removes the conflict check and updates parameter descriptions.

Review effort: Balanced

This branch was previously deployed

1 inactive deployment
github-app-auth — d49b25c4 Deployed Nov 2, 2026 by ZedingZhang via add_label #26869
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: Postgres vector stores reject settings instead of preferring an injected client

2 participants