Skip to content

refactor(helm): Remove unused query_scheduler.port from values.yaml (resolves #1812).#1815

Merged
junhaoliao merged 3 commits into
y-scope:mainfrom
junhaoliao:helm-remove-port
Dec 19, 2025
Merged

refactor(helm): Remove unused query_scheduler.port from values.yaml (resolves #1812).#1815
junhaoliao merged 3 commits into
y-scope:mainfrom
junhaoliao:helm-remove-port

Conversation

@junhaoliao

@junhaoliao junhaoliao commented Dec 19, 2025

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.

Resolves #1812 by removing the unused port: 7000 configuration from the query_scheduler section in values.yaml.

The query-scheduler port is an internal value that is hardcoded to 7000 in the templates (configmap.yaml and query-scheduler-deployment.yaml). Exposing it in values.yaml is misleading since changing the value would have no effect - the port is not user-facing and does not need to be configurable.

This aligns with the established pattern where internal services (query-scheduler, reducer) have hardcoded ports, while only user-facing and sbin-accessible services have configurable ports.

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

cd tools/deployment/package-helm
./test.sh

# observed "All jobs completed and services are ready."

Summary by CodeRabbit

  • Chores
    • Bumped deployment chart version for this release.
    • Removed an explicit query scheduler port entry to allow configurable/default port handling, improving deployment flexibility.

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

@junhaoliao junhaoliao requested a review from a team as a code owner December 19, 2025 04:11
@coderabbitai

coderabbitai Bot commented Dec 19, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Chart version bumped and an unused query_scheduler port entry removed from Helm values; no other functional changes.

Changes

Cohort / File(s) Summary
Helm chart metadata
tools/deployment/package-helm/Chart.yaml
Version updated from 0.1.2-dev.7 to 0.1.2-dev.9
Helm values configuration
tools/deployment/package-helm/values.yaml
Removed unused port: 7000 under clpConfig.query_scheduler; other query_scheduler fields unchanged

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The pull request contains one out-of-scope change: updating the Helm chart version from 0.1.2-dev.7 to 0.1.2-dev.9 in Chart.yaml, which is unrelated to the stated objective of removing the query_scheduler.port configuration. Either revert the Chart.yaml version change to keep only the intended port removal, or document the version bump as part of a separate scope within this PR.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: removing the unused query_scheduler.port configuration from values.yaml, and clearly references the resolved issue #1812.
Linked Issues check ✅ Passed The pull request fully satisfies the requirements of linked issue #1812 by removing the unused query_scheduler.port field from values.yaml at the specified location.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 99f1a13 and 69e2029.

📒 Files selected for processing (1)
  • tools/deployment/package-helm/values.yaml (0 hunks)
💤 Files with no reviewable changes (1)
  • tools/deployment/package-helm/values.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: lint-check (ubuntu-24.04)
  • GitHub Check: lint-check (macos-15)
  • GitHub Check: check-generated

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.

sitaowang1998
sitaowang1998 previously approved these changes Dec 19, 2025
@junhaoliao junhaoliao merged commit 3870d8e into y-scope:main Dec 19, 2025
21 checks passed
@junhaoliao junhaoliao deleted the helm-remove-port branch December 19, 2025 06:51
davidlion pushed a commit to davidlion/clp that referenced this pull request Jan 17, 2026
junhaoliao added a commit to junhaoliao/clp that referenced this pull request May 17, 2026
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.

Remove unused query_scheduler.port configuration field from values.yaml

2 participants