output: azure_blob: compression with zstd update#2309
output: azure_blob: compression with zstd update#2309eschabell merged 1 commit intofluent:masterfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughUpdated Azure Blob output docs: Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 Tip CodeRabbit can suggest fixes for GitHub Check annotations.Configure the |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pipeline/outputs/azure_blob.md(1 hunks)
🔇 Additional comments (1)
pipeline/outputs/azure_blob.md (1)
29-30: Well-documented compression options and inheritance behavior.The documentation clearly explains:
- Both
compress(transport-level) andcompress_blob(blob-level) compression support, with zstd now available- How
compress_blobinherits the codec fromcompresswhen set, establishing clear precedence- The incompatibility constraint with
blob_type = appendblobThese clarifications significantly improve the documentation. Once the underlying code merges in fluent-bit/fluent-bit#11202, verify that the documented behavior matches the implementation.
|
this looks fine to me, writing-wise. I don't know why the vale test is being picky, it's not supposed to fail out on a spelling issue. Pat's question remains though so I won't stamp it at this time. |
|
@patrick-stephens can you suggest a change that would make sense for @nberlee to apply? |
For me I just want to know what happens with the misconfiguration, does it fail explicitly or silently ignore or do something else? I know it's not something changed but as part of the update we should try to improve and capture that. |
|
I will put this on draft, there is not a reason for review at this time as the feature is still not merged. |
Signed-off-by: Nico Berlee <nico.berlee@on2it.net>
|
Rebased and addressed the comment. Since the zstd PR in fluent/fluent-bit has just been merged, I’ll update the PR status and remove the draft flag. |
|
Thanks @nberlee for the PR, merging! |
Document the new zstd option for Azure Blob network compression and explain how compress_blob inherits its codec, clarifying precedence between transport and blob-level compression.
Summary by CodeRabbit