Skip to content

feat: configurable PostgreSQL connection (discrete fields, TLS, pool) - #1584

Open
dcoric wants to merge 20 commits into
finos:feat/postgresfrom
dcoric:feat/postgres-connection-config
Open

feat: configurable PostgreSQL connection (discrete fields, TLS, pool)#1584
dcoric wants to merge 20 commits into
finos:feat/postgresfrom
dcoric:feat/postgres-connection-config

Conversation

@dcoric

@dcoric dcoric commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Description

Extension of #1532 (the PostgreSQL sink), stacked on it. The diff below currently includes the #1532 changes. Once #1532 merges, this branch will be rebased onto main so only the connection-configuration changes remain, and the draft flag will be removed.

Makes the PostgreSQL sink's connection fully configurable, bringing it closer to parity with the mongo backend.

  • Discrete connection fields host, port, user, password, database, used when connectionString is not set. If those are not set either, the standard PGHOST / PGPORT / PGUSER / PGPASSWORD / PGDATABASE environment variables are read by the client.
  • ssl: true for TLS with default certificate verification, or an object of TLS options (rejectUnauthorized, ca, cert, key, ...).
  • pool: connection pool tuning (max, idleTimeoutMillis, connectionTimeoutMillis).

Precedence: connectionString (the config field, then GIT_PROXY_POSTGRES_CONNECTION_STRING) wins; otherwise the discrete fields; otherwise the PG* environment variables. ssl and pool apply in all cases. If none of these resolve to a connection, GitProxy refuses to start rather than silently defaulting to localhost.

Unit tests cover the precedence, discrete fields, env fallback, ssl (boolean and object) and pool options.

Related Issue

Resolves #1560

Checklist

General

Documentation

  • Documentation has been added/updated for any new features

Configuration

  • If configuration schema (config.schema.json) was modified:
    • TypeScript types regenerated (npm run generate-config-types)
    • Schema reference docs regenerated (npm run gen-schema-doc requires the external generate-schema-doc tool; needs running in an environment that has it)

Tests

  • Tests have been added/updated for new functionality
  • Unit tests pass (npm test)
  • Linting and formatting pass (npm run lint and npm run format:check)
  • Type checks pass (npm run check-types)

@netlify

netlify Bot commented Jun 10, 2026

Copy link
Copy Markdown

Deploy Preview for endearing-brigadeiros-63f9d0 ready!

Name Link
🔨 Latest commit 57c1595
🔍 Latest deploy log https://app.netlify.com/projects/endearing-brigadeiros-63f9d0/deploys/6a6731438f431f00083d1cf6
😎 Deploy Preview https://deploy-preview-1584.git-proxy.preview.finos.org
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.11663% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.56%. Comparing base (b44f5ee) to head (57c1595).
⚠️ Report is 190 commits behind head on feat/postgres.

Files with missing lines Patch % Lines
src/db/postgres/users.ts 92.98% 12 Missing ⚠️
src/service/index.ts 33.33% 8 Missing ⚠️
src/db/postgres/helper.ts 95.04% 5 Missing ⚠️
src/db/index.ts 42.85% 4 Missing ⚠️
src/db/postgres/repo.ts 95.55% 4 Missing ⚠️
src/db/postgres/pushes.ts 97.14% 3 Missing ⚠️
Additional details and impacted files
@@                Coverage Diff                @@
##           feat/postgres    #1584      +/-   ##
=================================================
+ Coverage          90.41%   90.56%   +0.15%     
=================================================
  Files                 87       92       +5     
  Lines               8661     9182     +521     
  Branches            1603     1711     +108     
=================================================
+ Hits                7831     8316     +485     
- Misses               799      835      +36     
  Partials              31       31              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

# Conflicts:
#	src/config/generated/config.ts
Comment thread src/db/postgres/helper.ts
* loudly rather than silently defaulting to `localhost`.
*/
const hasConnectionConfig = (db: DatabaseConfig): boolean =>
Boolean(db.connectionString || db.host || process.env.PGHOST);

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.

This only checks PGHOST, but the message says "or the PG* env variables". A setup with PGUSER/PGDATABASE and no PGHOST is valid but gets refused. Could the check also look at PGUSER/PGDATABASE/PGHOSTADDR so only the truly-empty case is blocked?

Comment thread src/db/postgres/helper.ts
*/
const buildPoolConfig = (db: DatabaseConfig): PoolConfig => {
const config: PoolConfig = {};
if (db.connectionString) {

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.

When both connectionString and the discrete fields are set, the discrete fields are silently ignored. It's documented as the intended precedence, but it's an easy operational trap (e.g. someone changes host to repoint the DB, nothing happens because an old connectionString is still winning with no signal). Worth a console.warn here noting the discrete fields are being ignored?

@dcoric
dcoric marked this pull request as ready for review August 24, 2026 10:20
@dcoric
dcoric requested a review from a team as a code owner August 24, 2026 10:20
@jescalada
jescalada force-pushed the feat/postgres-connection-config branch from 4fc5065 to 1d6a005 Compare August 26, 2026 01:35
Signed-off-by: Juan Escalada <97265671+jescalada@users.noreply.github.com>

@jescalada jescalada 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 after fixing up Fabio's comments 👍🏼

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PostgreSQL sink: connection configuration (split env vars, pool & TLS options)

3 participants