Skip to content

fix(clp-package): Ensure --count and --count-by-time arguments are mutually exclusive in sbin search scripts (fixes #1859).#1871

Merged
junhaoliao merged 1 commit into
y-scope:mainfrom
junhaoliao:search-flags
Jan 15, 2026
Merged

Conversation

@junhaoliao

@junhaoliao junhaoliao commented Jan 15, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

Description

Adds validation to reject using --count and --count-by-time together in search.sh, as these options are mutually exclusive. Previously, using both options together would cause a KeyError: 'group_tags' because the second option's AggregationConfig would overwrite the first one's configuration.

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.
    • TODO: add docs for the search script usages. Then there we should document the options are exclusive to each other.

Validation performed

  • Built the package:
task && cd build/clp-package && ./sbin/start-clp.sh
2026-01-15T14:21:03.613 INFO [controller] Started CLP.
  • Compressed sample logs:
./sbin/compress.sh --timestamp-key timestamp ~/samples/postgresql.jsonl
2026-01-15T14:21:11.565 INFO [compress] Compression job 1 submitted.
2026-01-15T14:21:13.569 INFO [compress] Compressed 392.84MB into 9.94MB (39.53x). Speed: 207.53MB/s.
2026-01-15T14:21:14.070 INFO [compress] Compression finished.
2026-01-15T14:21:14.071 INFO [compress] Compressed 392.84MB into 9.94MB (39.53x). Speed: 183.93MB/s.
  • Verified that using both --count and --count-by-time now fails with a clear error message:
./sbin/search.sh --count --count-by-time 1 "*"
2026-01-15T14:21:20.806 ERROR [search] --count and --count-by-time are mutually exclusive.
  • Verified that --count works individually:
./sbin/search.sh --count "*"
tags: [] count: 1000000
  • Verified that --count-by-time works individually:
./sbin/search.sh --count-by-time 60000 "*"

->

timestamp: 1679876760000 count: 1351
timestamp: 1679876820000 count: 130543
timestamp: 1679876880000 count: 257459
timestamp: 1679876940000 count: 221434
timestamp: 1679877000000 count: 194047
timestamp: 1679877060000 count: 127016
timestamp: 1679877120000 count: 68150

Summary by CodeRabbit

  • Bug Fixes
    • Search functionality now includes improved validation that prevents conflicting counting mode options from being used simultaneously, ensuring only one counting method is active at a time.

✏️ 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 15, 2026 14:33
@junhaoliao junhaoliao requested a review from gibber9809 January 15, 2026 14:33
@coderabbitai

coderabbitai Bot commented Jan 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Walkthrough

Adds mutual exclusivity validation to two CLI search script entry points. When both --count and --count-by-time arguments are provided simultaneously, the programs log an error and exit with code -1, preventing concurrent usage of these counting modes.

Changes

Cohort / File(s) Summary
CLI mutual exclusivity validation
components/clp-package-utils/clp_package_utils/scripts/native/search.py, components/clp-package-utils/clp_package_utils/scripts/search.py		 Added conditional check after argument parsing to validate that --count and --count-by-time are not both specified. If both arguments are present, logs error message and exits with code -1. Short-circuits processing before further configuration steps.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding mutual exclusivity validation for --count and --count-by-time arguments in search scripts, with reference to the issue being fixed.

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

✨ Finishing touches
  • 📝 Generate docstrings

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.

gibber9809
gibber9809 approved these changes Jan 15, 2026
View reviewed changes

@gibber9809 gibber9809 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. For the PR title how about:

fix(clp-package): Ensure `--count` and `--count-by-time` arguments are mutually exclusive in sbin search scripts (fixes #1859).

@junhaoliao junhaoliao changed the title fix(clp-package): Ensure --count and --count-by-time are mutually exclusive in sbin search scripts (fixes #1859). fix(clp-package): Ensure --count and --count-by-time arguments are mutually exclusive in sbin search scripts (fixes #1859). Jan 15, 2026
@junhaoliao junhaoliao merged commit 491e321 into y-scope:main Jan 15, 2026
23 checks passed
@junhaoliao junhaoliao deleted the search-flags branch January 15, 2026 15:25
davidlion pushed a commit to davidlion/clp that referenced this pull request Jan 17, 2026
@junhaoliao @davidlion
fix(clp-package): Ensure --count and --count-by-time arguments ar…
b3b9914 
…e mutually exclusive in sbin search scripts (fixes y-scope#1859). (y-scope#1871)
junhaoliao added a commit to junhaoliao/clp that referenced this pull request May 17, 2026
@junhaoliao @davidlion
fix(clp-package): Ensure --count and --count-by-time arguments ar…
7b23f48 
…e mutually exclusive in sbin search scripts (fixes y-scope#1859). (y-scope#1871)
junhaoliao added a commit to junhaoliao/clp that referenced this pull request May 17, 2026
@junhaoliao
fix(clp-package): Ensure --count and --count-by-time arguments ar…
f7731f6 
…e mutually exclusive in sbin search scripts (fixes y-scope#1859). (y-scope#1871)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gibber9809 gibber9809 gibber9809 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@junhaoliao @gibber9809