Skip to content

fix: hardcode pgvector port to 5432 until lcore fix - #46

Merged
Jdubrick merged 1 commit into
redhat-ai-dev:mainfrom
Jdubrick:fix-pgvector-jul31
Jul 31, 2026
Merged

fix: hardcode pgvector port to 5432 until lcore fix#46
Jdubrick merged 1 commit into
redhat-ai-dev:mainfrom
Jdubrick:fix-pgvector-jul31

Conversation

@Jdubrick

Copy link
Copy Markdown
Contributor

What does this PR do?:

  • Hardcodes the port to 5432 as LCORE is not parsing env vars properly as strings.
  • Note: This is temporary until I push a fix to LCORE

Which issue(s) this PR fixes:

N/A

PR acceptance criteria:

Testing and documentation do not need to be complete in order for this PR to be approved. We just need to ensure tracking issues are opened and linked to this PR, if they are not in the PR scope due to various constraints.

  • Tested and Verified

  • Documentation (READMEs, Product Docs, Blogs, Education Modules, etc.)

How to test changes / Special notes to the reviewer:

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Hardcode pgvector port to 5432 in GitOps manifest generator (temporary workaround)

🐞 Bug fix ⚙️ Configuration changes 🕐 Less than 5 minutes

Grey Divider

AI Description

• Hardcode pgvector port to 5432 to bypass LCORE env-var string parsing bug.
• Add TODO note to revert once LCORE parsing fix is available.
Diagram

graph TD
  A["generate-gitops-manifests.sh"] --> B["Generated GitOps YAML"] --> C["LCORE runtime"] --> D[("Postgres/pgvector")]
  subgraph Legend
    direction LR
    _script["Script"] ~~~ _file["Manifest file"] ~~~ _svc["Service"] ~~~ _db[("Database")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Fix LCORE env-var parsing and keep port configurable
  • ➕ Restores configurability for non-default Postgres ports
  • ➕ Avoids accumulating temporary hardcodes in deployment tooling
  • ➖ May take longer than needed for an urgent unblock
  • ➖ Requires release/rollout of LCORE changes
2. Emit an explicit YAML string tag for the env-driven port (e.g., !!str)
  • ➕ Keeps port configurable without changing LCORE
  • ➕ Localized change in manifest generation
  • ➖ Depends on how LCORE parses YAML tags; may not be honored
  • ➖ Adds YAML-specific complexity to the generator
3. Render config via a template step at runtime (init/entrypoint) instead of printing in script
  • ➕ More robust type handling and validation
  • ➕ Centralizes config rendering logic closer to runtime
  • ➖ Larger change surface (pods/entrypoints), not suitable as a quick fix
  • ➖ Introduces new moving parts for GitOps deployments

Recommendation: For an immediate unblock, this PR’s hardcode is acceptable as a temporary workaround, especially with the added TODO. However, it should be paired with a tracking issue and a near-term follow-up to fix LCORE parsing and revert to env-driven configuration to avoid locking deployments to port 5432.

Files changed (1) +1 / -1

Bug fix (1) +1 / -1
generate-gitops-manifests.shHardcode pgvector port to 5432 in generated config +1/-1

Hardcode pgvector port to 5432 in generated config

• Replaces the env-driven PGVECTOR_PORT emission with a fixed "5432" value. Adds an inline TODO indicating this should be reverted once LCORE env parsing is corrected.

scripts/generate-gitops-manifests.sh

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. PGVECTOR override removed 🐞 Bug ☼ Reliability
Description
scripts/generate-gitops-manifests.sh now always emits pgvector port "5432" and ignores any
PGVECTOR_PORT value. Deployments that require a non-default Postgres port will be unable to
configure it via env and may fail to connect.
Code

scripts/generate-gitops-manifests.sh[53]

+      print "        port: \"5432\"  # TODO: fix lcore parsing and revert back to env"
Relevance

●●● Strong

Team recently reverted hardcoded pgvector port, keeping ${env.PGVECTOR_PORT:=5432} quoting (PR #45);
defaults pattern enforced (PR #36).

PR-#45
PR-#36

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The generator still uses env-expansion for host/db/user/password, but the port line is now a literal
"5432", meaning PGVECTOR_PORT can no longer affect the rendered pgvector connection settings.

scripts/generate-gitops-manifests.sh[41-56]
PR-#36

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The GitOps manifest generator hardcodes the pgvector `port` to `5432`, removing the previously supported `${env.PGVECTOR_PORT:=5432}` override path.

### Issue Context
Other pgvector connection fields (`host`, `db`, `user`, `password`) are still generated using `${env...}` expressions, so `port` being fixed is an inconsistent and potentially breaking regression for any environment not using 5432.

### Fix Focus Areas
- scripts/generate-gitops-manifests.sh[41-56]

### Suggested fix
Update the generated port line to preserve configurability with a safe default, e.g.:
- `print "        port: ${env.PGVECTOR_PORT:=5432}"`

If the underlying parsing issue requires a numeric type, consider emitting it unquoted:
- `print "        port: ${env.PGVECTOR_PORT:=5432}"` (no surrounding YAML quotes on the value)

Keep any temporary notes as a comment in the script (not necessarily in the generated YAML) and, if possible, link to a tracking issue for reverting once the lcore fix lands.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

print " config:"
print " host: ${env.PGVECTOR_HOST:=lightspeed-postgres-svc.lightspeed-postgres.svc.cluster.local}"
print " port: \"${env.PGVECTOR_PORT:=5432}\""
print " port: \"5432\" # TODO: fix lcore parsing and revert back to env"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Pgvector override removed 🐞 Bug ☼ Reliability

scripts/generate-gitops-manifests.sh now always emits pgvector port "5432" and ignores any
PGVECTOR_PORT value. Deployments that require a non-default Postgres port will be unable to
configure it via env and may fail to connect.
Agent Prompt
### Issue description
The GitOps manifest generator hardcodes the pgvector `port` to `5432`, removing the previously supported `${env.PGVECTOR_PORT:=5432}` override path.

### Issue Context
Other pgvector connection fields (`host`, `db`, `user`, `password`) are still generated using `${env...}` expressions, so `port` being fixed is an inconsistent and potentially breaking regression for any environment not using 5432.

### Fix Focus Areas
- scripts/generate-gitops-manifests.sh[41-56]

### Suggested fix
Update the generated port line to preserve configurability with a safe default, e.g.:
- `print "        port: ${env.PGVECTOR_PORT:=5432}"`

If the underlying parsing issue requires a numeric type, consider emitting it unquoted:
- `print "        port: ${env.PGVECTOR_PORT:=5432}"` (no surrounding YAML quotes on the value)

Keep any temporary notes as a comment in the script (not necessarily in the generated YAML) and, if possible, link to a tracking issue for reverting once the lcore fix lands.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@michael-valdron michael-valdron 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

@Jdubrick
Jdubrick merged commit 8e8ba99 into redhat-ai-dev:main Jul 31, 2026
3 checks passed
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.

2 participants