-
-
Notifications
You must be signed in to change notification settings - Fork 745
helm: increase default postgres resource preset #2221
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
WalkthroughThe Helm chart version for the "trigger" application was incremented from 4.0.0-beta.11 to 4.0.0-beta.12 in the Chart.yaml file. In the values.yaml file, a new key named 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
hosting/k8s/helm/Chart.yaml (1)
5-5
: Chart version bump looks good—remember ancillary updates (CHANGELOG / docs).
The 4.0.0-beta.12 increment is sem-ver-correct for a default-value change. Please double-check that accompanying docs/CHANGELOG entries reflect the new minimum Postgres resources so downstream users are aware.hosting/k8s/helm/values.yaml (1)
334-339
: resourcesPreset overrides empty resources block – consider commenting it outBitnami’s chart ignores the
resources:
object when aresourcesPreset
is supplied, so keeping both (even as{}
) may confuse operators. Commenting out the empty block makes intent clear:primary: persistence: enabled: true size: 10Gi resourcesPreset: "small" - resources: {} + # resources: {} + # When you need custom requests/limits, uncomment above and remove `resourcesPreset`.Please confirm locally that the
small
preset is picked up by running:helm template test ./hosting/k8s/helm -s templates/postgresql/**/*.yaml | grep -A3 "resources:"and verifying the generated pod spec includes the expected requests/limits from the
small
preset.• hosting/k8s/helm/values.yaml:334-339
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hosting/k8s/helm/Chart.yaml
(1 hunks)hosting/k8s/helm/values.yaml
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/Chart.yaml:1-18
Timestamp: 2025-06-25T13:18:44.103Z
Learning: For the Trigger.dev Helm chart, the chart name should be "trigger" (not "trigger-v4") since this is the first official chart release. Helper templates should use the actual chart name from .Chart.Name rather than hardcoded prefixes.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/values.yaml:22-51
Timestamp: 2025-06-25T13:20:17.174Z
Learning: In the Trigger.dev Helm chart values.yaml, the maintainer prefers to use explicit comprehensive warnings for security-sensitive default values rather than implementing secure-by-default behavior that would fail installation. The project uses deterministic default secrets with clear "TESTING ONLY" warnings and instructions for production deployment.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/templates/extra-manifests.yaml:1-4
Timestamp: 2025-06-25T13:18:04.827Z
Learning: In the Trigger.dev v4 Helm chart, the user prefers to rely on documentation and examples in values files rather than implementing defensive coding in templates, particularly for features like extraManifests where proper usage is documented.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/values-production-example.yaml:95-102
Timestamp: 2025-06-25T14:14:11.965Z
Learning: In the Trigger.dev Helm chart production examples, the maintainer prefers to include initial/bootstrap credentials with clear warnings that they should be changed after first login, rather than requiring external secret references that could complicate initial setup. This follows their pattern of providing working examples with explicit security guidance.
hosting/k8s/helm/Chart.yaml (9)
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/Chart.yaml:1-18
Timestamp: 2025-06-25T13:18:44.103Z
Learning: For the Trigger.dev Helm chart, the chart name should be "trigger" (not "trigger-v4") since this is the first official chart release. Helper templates should use the actual chart name from .Chart.Name rather than hardcoded prefixes.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/values.yaml:22-51
Timestamp: 2025-06-25T13:20:17.174Z
Learning: In the Trigger.dev Helm chart values.yaml, the maintainer prefers to use explicit comprehensive warnings for security-sensitive default values rather than implementing secure-by-default behavior that would fail installation. The project uses deterministic default secrets with clear "TESTING ONLY" warnings and instructions for production deployment.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/templates/extra-manifests.yaml:1-4
Timestamp: 2025-06-25T13:18:04.827Z
Learning: In the Trigger.dev v4 Helm chart, the user prefers to rely on documentation and examples in values files rather than implementing defensive coding in templates, particularly for features like extraManifests where proper usage is documented.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/values-production-example.yaml:95-102
Timestamp: 2025-06-25T14:14:11.965Z
Learning: In the Trigger.dev Helm chart production examples, the maintainer prefers to include initial/bootstrap credentials with clear warnings that they should be changed after first login, rather than requiring external secret references that could complicate initial setup. This follows their pattern of providing working examples with explicit security guidance.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.494Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : You MUST use `@trigger.dev/sdk/v3` when writing Trigger.dev tasks.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.495Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Each Trigger.dev task must have a unique `id` within your project.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2155
File: docs/docs.json:179-183
Timestamp: 2025-06-06T16:54:23.316Z
Learning: In the docs.json configuration for the Trigger.dev documentation (Mintlify system), both "tags": ["v4"] and "tag": "v4" properties can be used together and work correctly, even though this behavior is undocumented and may not work in local development environments.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.495Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Schema tasks must use `schemaTask` from `@trigger.dev/sdk/v3` and validate payloads using a schema (e.g., Zod).
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.495Z
Learning: Applies to trigger.config.ts : Build extensions for Trigger.dev must be configured in the `build` property of `trigger.config.ts` using provided or custom extensions.
🔇 Additional comments (1)
hosting/k8s/helm/values.yaml (1)
334-339
: AI-summary location mismatch (underprimary.persistence
).
The change was applied atprimary.resourcesPreset
, not insideprimary.persistence
. No action needed—just flagging the discrepancy.Likely an incorrect or invalid review comment.
The prior
nano
preset sets a very low memory limit causing OOM errors even while testing.