Skip to content

feat(clients/python): bring driver to parity with Go (API, tests, packaging)#82

Merged
NikolayS merged 7 commits intomainfrom
chore/python-driver-parity-and-tests
Apr 30, 2026
Merged

feat(clients/python): bring driver to parity with Go (API, tests, packaging)#82
NikolayS merged 7 commits intomainfrom
chore/python-driver-parity-and-tests

Conversation

@NikolayS
Copy link
Copy Markdown
Owner

Brings the Python driver to feature-parity with the Go client and adds a thorough integration test suite.

Audit (Go ↔ Python)

Capability Go driver Python (before) Python (this PR)
connect(dsn) factory Connect ❌ user passes conn pgque.connect
Close() / context manager close() + with
Send ✅ + Event support
SendBatch ✅ (Python ahead)
Receive
Ack
Nack
Consumer ✅ decorator API ✅ unchanged + tests
Wrapped errors fmt.Errorf ❌ raw psycopg PgqueError tree
Event / Message types structs only Message both

Bug list

Severity Where Issue Status
MED Message.payload: str psycopg auto-decodes jsonb to dict/list; type was wrong Fixed: payload: Any
MED No connect() factory users had to manage psycopg conn manually Fixed
MED No error wrapping bad DSN raised raw psycopg.OperationalError Fixed: PgqueConnectionError
LOW version = 0.1.0 mismatch with the SQL release cadence Fixed: 0.2.0
LOW No LICENSE/README.md not PyPI-ready Fixed
LOW Send rejected list payloads only dict was JSON-encoded Fixed: dict and list

(Note for the parent agent: while reading the SQL, I noticed clients/go/pgque.go Nack() builds ROW($2..$12) — 11 fields — but pgque.message is 10 fields and only 10 args are passed. Likely a runtime error in the Go path; outside this PR's scope.)

Per-fix breakdown

  • pgque/__init__.py — re-export connect, Event, error classes; add __version__.
  • pgque/client.py — add connect() factory, close(), context-manager protocol, error wrapping, Event unpacking in send, list handling.
  • pgque/types.pyMessage.payload: Any; new Event dataclass.
  • pgque/errors.py — new module with PgqueError + PgqueConnectionError / PgqueQueueNotFound / PgqueBatchNotFound / PgqueConsumerNotFound.
  • pgque/consumer.py — unchanged in this PR (already solid; gets new tests).
  • pyproject.toml — bump to 0.2.0, add readme, authors, keywords, classifiers, [project.urls], package-find config.
  • LICENSE — Apache-2.0 (copied from repo root).
  • README.md — install + 30-line quickstart + test instructions.

Tests

8 test files, ~30 tests. All env-gated via PGQUE_TEST_DSN; skipped when unset. Per-test unique queue + consumer names (random suffix), auto-teardown fixture.

File Coverage
test_connect.py factory, context manager, bad DSN, external conn, autocommit
test_send.py int return, type kwarg, Event object, str/None payloads, unicode, large payload, batch ordering, missing-queue error
test_receive.py empty when no tick, fields populated after tick, ack advances, max_messages cap, type preservation, timestamp round-trip
test_nack.py retry-queue routing, DLQ at max retries, invalid batch raises
test_consumer.py dispatch by event type, * default handler, handler error → nack, stop() returns promptly
test_concurrency.py 4 producers × 25 sends each, no event-id collisions
test_smoke.py end-to-end: connect → subscribe → send → tick → receive → ack

Run locally

PGQUE_TEST_DSN=postgresql://postgres:pgque_test@localhost/pgque_test \
    pytest clients/python/tests

Test plan

  • python3 -m py_compile passes on every file in pgque/ and tests/
  • pip install -e .[dev] && pytest clients/python/tests passes against PG14–18 (CI)
  • Reviewer eyeballs Event API choice before merge

🤖 Generated with Claude Code

NikolayS and others added 2 commits April 29, 2026 18:59
…kaging)

Brings the Python driver to feature-parity with the Go client and adds a
thorough integration test suite.

API:
- pgque.connect(dsn) factory + Client.close() lifecycle
- PgqueClient as context manager
- Wrapped error hierarchy: PgqueError + Connection/QueueNotFound/BatchNotFound
- Event dataclass for client.send convenience
- Message.payload typed as Any (psycopg auto-decodes jsonb)
- Send/SendBatch handle dict/list/None/Event payloads consistently

Tests:
- 30+ tests across send, receive, ack, nack, consumer, concurrency, connect
- Per-test unique queue + consumer names; auto teardown via fixture
- Env-gated via PGQUE_TEST_DSN; pytest.skip when absent

Packaging:
- Bump version 0.1.0 -> 0.2.0 (matches SQL release)
- Add LICENSE, README.md (install + quickstart + test instructions)
- Project URLs, classifiers, authors metadata

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sed pytest-asyncio

PEP 639: "License :: OSI Approved :: Apache Software License" classifier conflicts with the SPDX "license = \"Apache-2.0\"" expression. setuptools 68+ rejects this combination — was failing client-smoke CI. Also remove pytest-asyncio from dev deps; the driver is sync (psycopg3) and no test uses asyncio.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@NikolayS
Copy link
Copy Markdown
Owner Author

REV Review

CI: ⚠️ client-smoke was failing on the deprecated License :: classifier (PEP 639 / setuptools 68+ now rejects classifier + SPDX license = "..." together). Polish commit 333e10b removes that classifier and the unused pytest-asyncio dev dep — CI should re-run green. All other checks were green (test 14/15/16/17/18, verify, claude-review ✅).

Functional evidence: ast.parse() clean, package import structure validated. SQL is fully parameterized (psycopg %s and sql.Identifier for the LISTEN channel name — no injection risk). Nack builds a 10-placeholder ROW(...)::pgque.message correctly (matching the SQL type's 10 fields). Live integration tests env-gated via PGQUE_TEST_DSN; not run in this review.

Verdict: READY FOR USER REVIEW (post-polish)

Blocking

  • License :: OSI Approved :: Apache Software License classifier conflicts with license = "Apache-2.0" SPDX expression — fixed in 333e10b.

Non-blocking

  • tests/conftest.py:setup_queue cleanup catches Exception silently (except Exception: pass after rollback). On a real cleanup failure the test fails with a confusing prior-exception trace and no info about why teardown failed. Add at minimum import logging; logging.warning("setup_queue cleanup failed: %s", e) so debugging is possible.
  • Message.payload: Any — relies on psycopg3 auto-decoding jsonb. There IS a test_send.py round-trip test that should exercise this; worth eyeballing once a PGQUE_TEST_DSN is available.

Potential

  • test_concurrency.py — 4 worker threads × 25 sends. Verify each worker has its own psycopg.Connection (not shared), since psycopg connections are not thread-safe. If threads share a single Client.conn, this is a real bug.
  • Client.close() semantics — what happens on double-close? __exit__ reliable on exception inside with block? (Standard psycopg with is fine; just confirm Client.close() is idempotent.)
  • Send and Nack SQL construction — select pgque.send(%s, %s, %s::jsonb) (3 args) vs select pgque.send(%s, %s::jsonb) (2 args, type defaulted). Verify Send correctly picks the 2-arg form when Event.type is empty/None.

Anti-leak: ✅ no GitLab/AR/sahmed mentions in the diff.


REV review (security, bugs, tests, guidelines, docs). SOC2 items skipped per project policy.

NikolayS and others added 5 commits April 29, 2026 20:50
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@NikolayS
Copy link
Copy Markdown
Owner Author

REV Review (Round 2)

CI: All green (test 14/15/16/17/18, verify, client-smoke, claude-review).

Round 1 findings status:

  • NB1 (conftest cleanup logging) - addressed in 729265a: import logging + logging.warning("setup_queue cleanup failed: %s", e) followed by conn.rollback(). Correct and minimal.
  • NB2 (jsonb round-trip) - addressed in a7721ec: parametrized over dict / list / JSON-string / JSON-number / JSON-null. Solid coverage. Test asserts native Python types come back via psycopg3 auto-decode.
  • P1 (thread-local conns) - confirmed not a bug, no commit needed. test_concurrency.py:_producer does with pgque.connect(dsn) as client inside each worker, so every thread gets its own psycopg.Connection. No sharing.
  • P2 (Client.close() idempotency) - addressed in 6e1c26b. Test calls close() twice; the implementation guards with if self._owns_conn and not self.conn.closed, which is correct because psycopg.Connection.closed flips synchronously inside close(). Standard pytest single-thread context, so no race concern at this layer.
  • P3 (type != \"default\" gate) - addressed via red/green TDD. Red 36e7fa7 adds test_send_sql_form_selection parametrized over None / \"\" / \"default\" / \"custom\". Green a7cba91 flips if type != \"default\": to if type and type != \"default\":. Real bug, real fix. TDD discipline followed (CLAUDE.md key design rule feat: add PgQ v3.5.1 as git submodule #6).

Verdict: READY FOR USER REVIEW

Blocking

None.

Non-blocking

  • P3 fix - whitespace type not normalized. if type and type != \"default\": covers None, \"\", and \"default\". It does NOT cover whitespace-only strings like \" \" (truthy, != \"default\", so falls into 3-arg form and inserts a literal whitespace event type). Likely caller-error rather than a driver bug; SPECx does not require driver-side normalization. Worth a one-line comment in send() explaining the contract ("empty string and None both mean default; type is otherwise passed through verbatim") so future readers don't try to "fix" it.
  • type: str = \"default\" annotation lies. The new parametrized test passes type=None explicitly, but the signature is type: str = \"default\". A strict mypy run would flag callers that pass None. Either widen to Optional[str] = \"default\" or document that None is also accepted at runtime. Cosmetic.
  • type shadows builtin in send(). Pre-existing, not introduced by P3 fix. Fine for now; consider event_type in a future API revision.

Potential

  • _wrap_sql_error does substring matching on error text (\"queue not found\", \"batch not found\"). This is fragile across PG locales / pgque error message wording changes. Tracking via a SQLSTATE map (or e.diag.message_primary) would be more robust. Not a regression from this PR, but worth noting since the new error hierarchy depends on it.
  • Event.extra: dict[str, str] is unused by send() (only payload and type are unpacked). Either thread extra through to pgque.insert_event(... ev_extra1..4) or drop the field until it's wired. Dead-feature risk.

Anti-leak: clean (no GitLab/AR/sahmed/WI/R8 mentions in diff, commits, or PR body).

Functional evidence: SQL fully parameterized (%s bindings, sql.Identifier for the LISTEN channel name). Nack builds the 10-field ROW(...)::pgque.message correctly (matches the SQL composite type). pyproject.toml SPDX license = \"Apache-2.0\" no longer conflicts with classifiers (deprecated License :: row removed in 333e10b). README install + 30-line quickstart present and accurate. __version__ = \"0.2.0\" matches pyproject.toml.

Counts: 5 commits since round 1; ~32 added test functions across 8 test files.


REV review (security, bug hunter, test analyzer, guidelines, docs). SOC2 items skipped per project policy.

@NikolayS NikolayS merged commit 6b71df3 into main Apr 30, 2026
8 checks passed
@NikolayS NikolayS deleted the chore/python-driver-parity-and-tests branch April 30, 2026 09:30
NikolayS added a commit that referenced this pull request Apr 30, 2026
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
PR #82 (Python) and #83 (TypeScript) are now merged on main.
Drop --ignore flags for test_consumer.py/test_smoke.py (Python)
and --passWithNoTests (TypeScript); all real tests now run.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
PR #82 (Python) and #83 (TypeScript) are now merged on main.
Drop --ignore flags for test_consumer.py/test_smoke.py (Python)
and --passWithNoTests (TypeScript); all real tests now run.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
PR #82 (Python) and #83 (TypeScript) are now merged on main.
Drop --ignore flags for test_consumer.py/test_smoke.py (Python)
and --passWithNoTests (TypeScript); all real tests now run.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NikolayS added a commit that referenced this pull request Apr 30, 2026
PR #82 (Python) and #83 (TypeScript) are now merged on main.
Drop --ignore flags for test_consumer.py/test_smoke.py (Python)
and --passWithNoTests (TypeScript); all real tests now run.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant