Skip to content

LCORE-5372: Select psycopg2 explicitly for PostgreSQL database engines - #3107

Merged
tisnik merged 1 commit into
lightspeed-core:mainfrom
dprince:sqlalchemy_fix
Nov 2, 2026
Merged

tisnik merged 1 commit into
lightspeed-core:mainfrom
dprince:sqlalchemy_fix

Conversation

@dprince

@dprince dprince commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

SQLAlchemy 2.1 changed the default driver for postgresql:// URLs from psycopg2 to psycopg (version 3). LCS depends on psycopg2-binary, so the implicit driver selection now fails with ModuleNotFoundError for psycopg when initializing a PostgreSQL-backed runtime database.

Use postgresql+psycopg2:// to select the existing dependency explicitly and update the database unit test to expect that URL.

This fixes startup for PostgreSQL-backed deployments, including those managed by the OpenStack Lightspeed operator, which configures the LCS runtime database to use PostgreSQL. SQLite-backed deployments do not exercise this path.

Jira: LCORE-5372

Summary by CodeRabbit

  • Bug Fixes
    • PostgreSQL database connections now explicitly use the psycopg2 driver. Existing connection settings are unchanged, so this update does not alter the configured database parameters.

SQLAlchemy 2.1 changed the default driver for postgresql:// URLs from
psycopg2 to psycopg (version 3). LCS depends on psycopg2-binary, so the
implicit driver selection now fails with ModuleNotFoundError for psycopg
when initializing a PostgreSQL-backed runtime database.

Use postgresql+psycopg2:// to select the existing dependency explicitly
and update the database unit test to expect that URL.

This fixes startup for PostgreSQL-backed deployments, including those
managed by the OpenStack Lightspeed operator, which configures the LCS
runtime database to use PostgreSQL. SQLite-backed deployments do not
exercise this path.

Jira: LCORE-5372
@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 51c1c3e and 08c198c.

Files selected for processing: 2
  • src/app/database.py
  • tests/unit/app/test_database.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Recent review details
Context from checks skipped due to timeout: 33
  • GitHub Check: E2E: library / ci / shields
  • GitHub Check: E2E: server / ci / tls
  • GitHub Check: E2E: library / ci / authorized
  • GitHub Check: E2E: library / ci / skills
  • GitHub Check: E2E: server / ci / shields
  • GitHub Check: E2E: server / ci / other
  • GitHub Check: E2E: library / ci / other
  • GitHub Check: E2E: server / ci / authorized
  • GitHub Check: E2E: library / ci / mcp
  • GitHub Check: E2E: server / ci / skills
  • GitHub Check: E2E: library / ci / default
  • GitHub Check: E2E: server / ci / rbac
  • GitHub Check: E2E: server / ci / mcp
  • GitHub Check: E2E: library / ci / rbac
  • GitHub Check: E2E: server / ci / default
  • GitHub Check: integration_tests (3.14)
  • GitHub Check: integration_tests (3.12)
  • GitHub Check: integration_tests (3.13)
  • GitHub Check: radon
  • GitHub Check: black
  • GitHub Check: Pylinter
  • GitHub Check: build-pr
  • GitHub Check: check_dependencies
  • GitHub Check: unit_tests (3.13)
  • GitHub Check: spectral
  • GitHub Check: unit_tests (3.14)
  • GitHub Check: bandit
  • GitHub Check: unit_tests (3.12)
  • GitHub Check: mypy
  • GitHub Check: Pyright
  • GitHub Check: dispatch / Route
  • GitHub Check: dispatch / Harness dispatch
  • GitHub Check: Konflux kflux-prd-rh02
CI failures not shown inline: 2

GitHub Actions: PR Title Checker / 0_check.txt: Select psycopg2 explicitly for PostgreSQL database engines

Conclusion: failure

View job details

##[group]Run thehanimo/pr-title-checker@v1.4.3
 with:
   GITHUB_***REDACTED_SECRET_ASSIGNMENT***
   pass_on_octokit_error: false
   configuration_path: .github/pr-title-checker-config.json
 ##[endgroup]
 Using config file .github/pr-title-checker-config.json from repo lightspeed-core/lightspeed-stack [ref: 51c1c3ed668260ddd5edf6024c488d83f39ce24d]
 (node:2418) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
 (Use `node --trace-deprecation ...` to show where the warning was created)
 (node:2418) [DEP0169] DeprecationWarning: `url.parse()` behavior is not standardized and prone to errors that have security implications. Use the WHATWG URL API instead. CVEs are not issued for `url.parse()` vulnerabilities.
 Creating label (title needs formatting)...
 Label (title needs formatting) already created.
 Adding label (title needs formatting) to PR...
 HttpError: Resource not accessible by integration
 ##[error]Failed to add label (title needs formatting) to PR

GitHub Actions: PR Title Checker / check: Select psycopg2 explicitly for PostgreSQL database engines

Conclusion: failure

View job details

##[group]Run thehanimo/pr-title-checker@v1.4.3
 with:
   GITHUB_***REDACTED_SECRET_ASSIGNMENT***
   pass_on_octokit_error: false
   configuration_path: .github/pr-title-checker-config.json
 ##[endgroup]
 Using config file .github/pr-title-checker-config.json from repo lightspeed-core/lightspeed-stack [ref: 51c1c3ed668260ddd5edf6024c488d83f39ce24d]
 (node:2418) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
 (Use `node --trace-deprecation ...` to show where the warning was created)
 (node:2418) [DEP0169] DeprecationWarning: `url.parse()` behavior is not standardized and prone to errors that have security implications. Use the WHATWG URL API instead. CVEs are not issued for `url.parse()` vulnerabilities.
 Creating label (title needs formatting)...
 Label (title needs formatting) already created.
 Adding label (title needs formatting) to PR...
 HttpError: Resource not accessible by integration
 ##[error]Failed to add label (title needs formatting) to PR
Additional context used
Code guidelines: 1
AGENTS.md — auto-discovered
Path-based instructions: 2
Source excerpt: **Required**: Use pytest for all unit and integration tests Source excerpt: **Forbidden**: Do not use unittest - pytest is the standard for this project Source excerpt: **Coverage**: Unit tests require 60% coverage, integrat...

CodeRabbit inference engine (AGENTS.md)

Files:

  • tests/unit/app/test_database.py
Source excerpt: **Fixtures**: Use `conftest.py` for shared fixtures Source excerpt: **Mocking**: `pytest-mock` for AsyncMock objects Source excerpt: **Async tests**: Use marker `pytest.mark.asyncio`

CodeRabbit inference engine (AGENTS.md)

Files:

  • tests/unit/app/test_database.py
Additional comments: 2
src/app/database.py: 1

132-132: LGTM!

tests/unit/app/test_database.py: 1

185-185: LGTM!

Walkthrough

The PostgreSQL connection URL now explicitly selects the psycopg2 driver. The unit test expects the updated URL scheme and continues to check the remaining parameters.

Changes

PostgreSQL driver URL

Layer / File(s) Summary
Set and verify the PostgreSQL driver
src/app/database.py, tests/unit/app/test_database.py
The connection URL specifies the psycopg2 driver. The test expects the updated scheme and checks the remaining URL parameters.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Suggested reviewers: omertuc, tisnik

Merge Risk: Minimal · up to 08c19

The PostgreSQL driver change matches the supported runtime dependency and does not introduce an evident merge-blocking risk.

Pre-merge checks: Passed 7
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Performance And Algorithmic Complexity Passed PASS. The pull request only changes the PostgreSQL SQLAlchemy driver string and its unit-test expectation. It adds no loops, API or database query patterns, list operations, caches, buffers, or endpoi…
Security And Secret Handling Passed No security violation is introduced. The production diff changes only the SQLAlchemy driver prefix from postgresql:// to postgresql+psycopg2:// at src/app/database.py:132; it does not add loggin…
Title check Passed The title names the main change: PostgreSQL database engines now select the psycopg2 driver explicitly. The changeset does this by changing the connection URL to postgresql+psycopg2 in src/app/databas…
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.

Full details: Performance And Algorithmic Complexity

Explanation

PASS. The pull request only changes the PostgreSQL SQLAlchemy driver string and its unit-test expectation. It adds no loops, API or database query patterns, list operations, caches, buffers, or endpoint changes. No meaningful performance regression is introduced.

Full details: Security And Secret Handling

Explanation

No security violation is introduced. The production diff changes only the SQLAlchemy driver prefix from postgresql:// to postgresql+psycopg2:// at src/app/database.py:132; it does not add logging, endpoints, responses, secret storage, SQL construction, or filesystem handling. The password interpolation and existing schema operation are unchanged. The test change only updates the expected URL.

Full details: Title check

Explanation

The title names the main change: PostgreSQL database engines now select the psycopg2 driver explicitly. The changeset does this by changing the connection URL to postgresql+psycopg2 in src/app/database.py and updating the matching unit test. The title is specific and matches the stated objective of avoiding the implicit psycopg (v3) default in SQLAlchemy 2.1.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
Simplify code
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@tisnik tisnik 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.

LGTM

@umago umago 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.

Thanks @dprince! It LGTM!

@tisnik

tisnik commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

/ok-to-test

@dprince dprince changed the title Select psycopg2 explicitly for PostgreSQL database engines LCORE-5372: Select psycopg2 explicitly for PostgreSQL database engines Nov 2, 2026

@lpiwowar lpiwowar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thank you!

@tisnik
tisnik merged commit 4cae323 into lightspeed-core:main Nov 2, 2026
108 of 112 checks passed
@fullsend-ai-retro

fullsend-ai-retro Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 4:59 PM UTC · Completed 5:12 PM UTC

Commit: 08c198c · View workflow run →

Runtime: pi · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.88

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #3107 — LCORE-5372: Select psycopg2 explicitly for PostgreSQL database engines

Workflow quality: Clean. This was a well-executed human-authored PR by @dprince — a 2-line bug fix for SQLAlchemy 2.1's change to the default PostgreSQL driver from psycopg2 to psycopg (v3). The fix explicitly selects postgresql+psycopg2:// in the connection URL, with a matching test update. Three human reviewers (tisnik, umago, lpiwowar) approved with LGTM. CodeRabbit rated it "Trivial" with no actionable findings. All CI passed. Merged same day (~4.5 hours after creation). Single commit, zero rework.

No fullsend agent involvement in this PR. No code, review, or fix agents ran. This was entirely a human workflow. There is nothing to assess for review quality, rework rate, or autonomy readiness.

Key finding — retro triggered despite config restriction. The .fullsend/config.yaml registers a retro agent with source: harness/retro.yaml, and the derived harness at .fullsend/harness/retro.yaml overrides the trigger to fire only on /fs-retro commands from humans. Despite this, the retro dispatched on the PR merge event. The Route job's bash routing hardcodes STAGE="retro" for closed PRs, and the CEL routing steps that would evaluate the derived harness trigger were skipped — so the bash stage passed through unchecked.

Already being addressed. PR #3112 (opened the same day, before this PR merged) switches the retro agent from the derived harness approach to an inline trigger: in config.yaml and deletes the harness overlay. PR #3070 does the same for the review agent. Once these PRs merge, both agents should respect the /fs-retro and /fs-review restrictions.

Evidence for existing platform issues:

  • fullsend-ai/fullsend#7719 ("Allow config.yaml to override harness CEL triggers"): This retro confirms the derived harness trigger override is not evaluated during dispatch routing, making the inline config override even more necessary.
  • fullsend-ai/fullsend#6297 ("Extend retro skip logic to merged PRs with zero fullsend agent involvement"): This PR had zero agent involvement — the retro consumed tokens analyzing a human-only workflow and found no agent-performance improvements.
  • fullsend-ai/fullsend#2902 ("Make harness-dispatch primary and remove bash routing"): The root cause is that bash routing hardcodes retro dispatch for closed PRs, and CEL-based dispatch (which would evaluate harness triggers) is not yet primary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants