Skip to content

fix(helm): Ensure log_ingestor is explicitly null in configmap when disabled (fixes #1881).#1882

Merged
junhaoliao merged 1 commit into
y-scope:mainfrom
junhaoliao:fix-helm-log-ingestor
Jan 19, 2026
Merged

fix(helm): Ensure log_ingestor is explicitly null in configmap when disabled (fixes #1881).#1882
junhaoliao merged 1 commit into
y-scope:mainfrom
junhaoliao:fix-helm-log-ingestor

Conversation

@junhaoliao

@junhaoliao junhaoliao commented Jan 19, 2026

Copy link
Copy Markdown
Member

Description

Note

This PR is part of the ongoing work for #1309. More PRs will be submitted until the Helm chart is complete and fully functional.

Sets the log_ingestor in tools/deployment/package-helm/templates/configmap.yaml as null when the log_ingestor is configured to be disabled (also null) in values.yaml.

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

Applied below changes to values.yaml:

diff --git a/tools/deployment/package-helm/values.yaml b/tools/deployment/package-helm/values.yaml
--- a/tools/deployment/package-helm/values.yaml	(revision 4d38a7892b43e86d8b93aa6b18e87e949b0ec2c2)
+++ b/tools/deployment/package-helm/values.yaml	(date 1768794180678)
@@ -88,16 +88,11 @@
 
 clpConfig:
   package:
-    storage_engine: "clp-s"
-    query_engine: "clp-s"
+    storage_engine: "clp"
+    query_engine: "clp"
 
   # API server config
-  api_server:
-    port: 30301
-    default_max_num_query_results: 1000
-    query_job_polling:
-      initial_backoff_ms: 100
-      max_backoff_ms: 5000
+  api_server: null
 
   # Location (e.g., directory) containing any logs you wish to compress. Must be reachable by all
   # workers.

Ran tools/deployment/package-helm/set-up-test.sh and observed "All jobs completed and services are ready."

Summary by CodeRabbit

Release Notes

  • Chores
    • Version bumped to 0.1.2-dev.19
    • Internal configuration enhancements for improved deployment management

✏️ Tip: You can customize this high-level summary in your review settings.

@junhaoliao junhaoliao requested a review from a team as a code owner January 19, 2026 03:49
@coderabbitai

coderabbitai Bot commented Jan 19, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Bumps Helm Chart version from "0.1.2-dev.18" to "0.1.2-dev.19" and adds a conditional else branch to the log_ingestor configuration block in the configmap template, setting the value to null when the condition is not met.

Changes

Cohort / File(s) Summary
Version Bump
tools/deployment/package-helm/Chart.yaml
Increments Chart version to "0.1.2-dev.19"
Template Configuration
tools/deployment/package-helm/templates/configmap.yaml
Adds else branch to log_ingestor block to render null value when condition is false

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: ensuring log_ingestor is explicitly null in the Helm configmap when disabled, and references the fixed issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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

Comment @coderabbitai help to get the list of available commands and usage tips.

@junhaoliao junhaoliao merged commit ed107fd into y-scope:main Jan 19, 2026
23 checks passed
@junhaoliao junhaoliao deleted the fix-helm-log-ingestor branch January 19, 2026 04:05
junhaoliao added a commit to junhaoliao/clp that referenced this pull request May 17, 2026
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