packaging: pass the elements to jq by pipes instead of command line - #12226
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe package catalog now applies a configurable minimum major version to paths, artifacts, and emitted versions. The package index script passes this setting to the AWK builder. Tests cover legacy versions, invalid input, and large catalogs. ChangesCatalog filtering and index generation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PackageIndex as generate-packages-index.sh
participant CatalogBuilder as build-catalog.awk
participant JSONBuilder as jq
PackageIndex->>CatalogBuilder: pass MIN_CATALOG_MAJOR
CatalogBuilder-->>PackageIndex: filtered package paths and versions
PackageIndex->>JSONBuilder: stream version rows
JSONBuilder-->>PackageIndex: build versions.json
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packaging/build-catalog.awk (1)
291-295: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply the configured catalog filter to Linux artifacts.
With
MIN_CATALOG_MAJOR=0, Line 291 accepts2.1.0and Line 295 callsnote_linux.note_linuxthen drops the RPM becauseversion_includes_linuxaccepts only 4.x and 5.x. This also drops Linux artifacts for future major versions such as 6.x. The documented permissive mode does not include all catalog artifacts.
packaging/build-catalog.awk#L291-L295: Remove the fixed 4.x/5.x gate from Linux artifact registration. Use the configuredversion_in_catalogresult.packaging/testing/test-packages-index.sh#L281-L286: Assert thatfluent-bit-2.1.0-1.x86_64.rpmis present in the2.1.0Linux artifacts inversions.json.As per coding guidelines, validate both success and failure paths, including invalid payloads, boundary sizes, and null or missing fields.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packaging/build-catalog.awk` around lines 291 - 295, The Linux artifact registration in note_linux must rely solely on version_in_catalog(v), removing the fixed 4.x/5.x filtering so configured permissive mode includes all catalog versions; update packaging/build-catalog.awk lines 291-295 accordingly. Add a test in packaging/testing/test-packages-index.sh lines 281-286 asserting fluent-bit-2.1.0-1.x86_64.rpm appears among the 2.1.0 Linux artifacts in versions.json.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packaging/build-catalog.awk`:
- Line 3: Update the initialization around min_catalog_major so
MIN_CATALOG_MAJOR is validated as a present, nonnegative integer before numeric
coercion; reject nonnumeric and negative values instead of allowing them to
become the permissive 0 setting. Add coverage for valid values, nonnumeric and
negative inputs, and relevant missing/null or boundary cases, verifying both
accepted and rejected paths.
In `@packaging/generate-packages-index.sh`:
- Around line 208-217: Update the build-catalog.awk invocation in
generate-packages-index.sh to remove the failure suppression and ensure a
nonzero awk exit status terminates the script before the filtered tree is used.
Preserve the existing empty-output handling only for successful filtering, and
validate both successful output and filter failures, including invalid or
missing catalog data and boundary-sized inputs.
---
Outside diff comments:
In `@packaging/build-catalog.awk`:
- Around line 291-295: The Linux artifact registration in note_linux must rely
solely on version_in_catalog(v), removing the fixed 4.x/5.x filtering so
configured permissive mode includes all catalog versions; update
packaging/build-catalog.awk lines 291-295 accordingly. Add a test in
packaging/testing/test-packages-index.sh lines 281-286 asserting
fluent-bit-2.1.0-1.x86_64.rpm appears among the 2.1.0 Linux artifacts in
versions.json.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 13dd62e4-153b-4abb-8b07-26f19a973e80
📒 Files selected for processing (3)
packaging/build-catalog.awkpackaging/generate-packages-index.shpackaging/testing/test-packages-index.sh
Signed-off-by: lecaros <lecaros@chronosphere.io>
78ef159 to
c4e6a37
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
2 participants
Pass the elements to jq by pipe instead of command line and reduce the versions to display in the index
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit
New Features
0.Tests