Skip to content

fix: postgres secret key - #299

Open
bastiaanv wants to merge 1 commit into
frappe:mainfrom
bastiaanv:main
Open

bastiaanv wants to merge 1 commit into
frappe:mainfrom
bastiaanv:main

Conversation

@bastiaanv

Copy link
Copy Markdown

Please provide enough information so that others can review your pull request:

fixing a typo in the postgres sercet key for Stateful postgres version:

Explain the details for making this change. What existing problem does the pull request solve?

The generated secret in my k8s is;

apiVersion: v1
kind: Secret
metadata:
  name: wbe-frappe
type: Opaque
stringData:
  postgres-password: ***

Note that this secret creates postgres-password instead of postgresql-password

@greptile-apps

greptile-apps Bot commented Sep 15, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

The key correction is consistent, but the repository’s regression-test requirement should be satisfied before merging.

Reviews (1) · Last reviewed commit: "fix: postgres secret key"

valueFrom:
secretKeyRef:
key: postgresql-password
key: postgres-password

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Missing PostgreSQL regression coverage

Both PostgreSQL site-creation jobs should be rendered in tests and checked for secretKeyRef.key: postgres-password. Current CI exercises only the single-site MariaDB branch, so this typo could recur undetected. This violates the repository requirement that bug fixes include regression coverage and must be addressed before merging.

Context Used: Guidelines for reviewing Frappe Framework applications. (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: erpnext/templates/job-create-site.yaml
Line: 157

Comment:
**Missing PostgreSQL regression coverage**

Both PostgreSQL site-creation jobs should be rendered in tests and checked for `secretKeyRef.key: postgres-password`. Current CI exercises only the single-site MariaDB branch, so this typo could recur undetected. This violates the repository requirement that bug fixes include regression coverage and must be addressed before merging.

**Context Used:** Guidelines for reviewing Frappe Framework applications. ([source](https://github.com/frappe/skills/blob/main/skills/quality-code-review/SKILL.md))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

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