Slim dependencies and ship a batteries-included default binary#944
Merged
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 ignored due to path filters (1)
📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR expands the default distribution feature bundle from ChangesBatteries-included distribution, rustls migration, and bech32 update
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 1
🧹 Nitpick comments (2)
docs/v2/installation/from_source.mdx (1)
32-32: 💤 Low valueConsider clarifying the OpenSSL prerequisite wording.
The current wording "OpenSSL (its only TLS backend); your platform's OpenSSL dev libraries" could be slightly clearer. Consider: "OpenSSL development libraries (headers and libs) — Kafka's only TLS backend".
This is a minor suggestion; the current wording is already acceptable.
🤖 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 `@docs/v2/installation/from_source.mdx` at line 32, In the kafka prerequisites table entry in docs/v2/installation/from_source.mdx, replace the description "OpenSSL (its only TLS backend); your platform's OpenSSL dev libraries" with "OpenSSL development libraries (headers and libs) — Kafka's only TLS backend" to improve clarity by explicitly mentioning what development libraries include (headers and libs) and using an em dash for better readability while maintaining the same informational content.docs/v2/installation/binary_release.mdx (1)
15-15: 💤 Low valueConsider adding a note about the source build link earlier.
The phrase "require a source build" appears inline. Consider whether users would benefit from this link being more prominent, or if the current placement is intentional to keep focus on what's included.
🤖 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 `@docs/v2/installation/binary_release.mdx` at line 15, The link to the source build documentation at `/v2/installation/from_source` is currently embedded inline within a sentence about integrations that require special build environments. To improve readability and ensure users don't miss this important information, consider adding a more prominent note or callout earlier in the document (before or at the beginning of the binary_release.mdx section) that highlights the source build requirement and provides a clear, visible reference to that documentation. This would make the distinction between binary and source installations more obvious to readers.
🤖 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 `@docs/v2/installation/binary_release.mdx`:
- Around line 7-21: The documentation accurately lists Redis as an included sink
in the pre-built binaries, but the Cargo.toml configuration doesn't have a
corresponding feature definition to support this claim. Add a `redis =
["redis"]` entry to the `[features]` section of Cargo.toml to match the existing
`optional = true` dependency declaration and the redis entry in the
`[package.metadata.dist]` features configuration. This will ensure cargo-dist
properly recognizes and includes Redis in the build process.
---
Nitpick comments:
In `@docs/v2/installation/binary_release.mdx`:
- Line 15: The link to the source build documentation at
`/v2/installation/from_source` is currently embedded inline within a sentence
about integrations that require special build environments. To improve
readability and ensure users don't miss this important information, consider
adding a more prominent note or callout earlier in the document (before or at
the beginning of the binary_release.mdx section) that highlights the source
build requirement and provides a clear, visible reference to that documentation.
This would make the distinction between binary and source installations more
obvious to readers.
In `@docs/v2/installation/from_source.mdx`:
- Line 32: In the kafka prerequisites table entry in
docs/v2/installation/from_source.mdx, replace the description "OpenSSL (its only
TLS backend); your platform's OpenSSL dev libraries" with "OpenSSL development
libraries (headers and libs) — Kafka's only TLS backend" to improve clarity by
explicitly mentioning what development libraries include (headers and libs) and
using an em dash for better readability while maintaining the same informational
content.
🪄 Autofix (Beta)
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
Run ID: 7001eda7-8ca5-43c9-8dd0-d7c509f8448d
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
Cargo.tomldocs/v2/installation/binary_release.mdxdocs/v2/installation/from_source.mdxsrc/filters/select/eval/cip14.rssrc/filters/select/eval/serde_ext.rs
Comment on lines
+7
to
+21
| ## What's included | ||
|
|
||
| The pre-built binaries are **batteries-included**: every integration that builds cleanly across all release targets ships by default, so you rarely need to compile a custom build. | ||
|
|
||
| - **Sources:** node-to-node (`n2n`), node-to-client (`n2c`), UTxO RPC (`u5c`), Hydra (`hydra`), AWS S3 (`aws`) | ||
| - **Sinks:** terminal, stdout, file rotate, webhook, Elasticsearch, Redis, RabbitMQ, SQL (SQLite / Postgres), AWS (SQS / Lambda / S3), GCP (Pub/Sub / Cloud Functions) | ||
| - **Filters:** all built-in filters (select, split block, JSON, legacy v1, etc.) | ||
|
|
||
| Only integrations that need a special build environment are left out and require a [source build](/v2/installation/from_source): | ||
|
|
||
| - **`kafka`** — its only TLS backend is OpenSSL (a system C library), unlike the rest of the binary which is pure-Rust TLS | ||
| - **`zeromq`** — links a system `libzmq` | ||
| - **`mithril`** — pulls the GMP C library, which is unsupported on Windows (MSVC) | ||
| - **`wasm`** — the WASM plugin runtime (large, and itself a plugin host) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify n2n, n2c, and basic sinks are not feature-gated
# Check if n2n and n2c sources are behind feature flags
echo "=== Checking if n2n/n2c are feature-gated ==="
rg -nP --type=toml 'features?\s*=.*\b(n2n|n2c)\b' Cargo.toml
# Check if basic sinks (terminal, stdout, webhook) are feature-gated
echo -e "\n=== Checking if basic sinks are feature-gated ==="
rg -nP --type=toml 'features?\s*=.*(terminal|stdout|webhook)' Cargo.toml
# Look for the feature definitions section
echo -e "\n=== Feature definitions ==="
rg -nA 50 '^\[features\]' Cargo.toml | head -100Repository: txpipe/oura
Length of output: 2737
🏁 Script executed:
# First, let's look at the complete dependencies section
echo "=== Complete Cargo.toml dependencies and optional markers ==="
rg -A 200 '^\[dependencies\]' Cargo.toml | grep -E '(n2n|n2c|terminal|stdout|webhook|file|redis|optional)' | head -50Repository: txpipe/oura
Length of output: 1716
🏁 Script executed:
# Search for where n2n and n2c are actually implemented/used
echo "=== Searching for n2n and n2c references ==="
fd -t f -e rs -e toml | xargs rg -l 'n2n|n2c' | head -10Repository: txpipe/oura
Length of output: 219
🏁 Script executed:
# Look for sink implementations
echo "=== Searching for terminal, stdout, webhook sink files ==="
fd -t f '(terminal|stdout|webhook|file)' --type f | grep -i sinkRepository: txpipe/oura
Length of output: 341
🏁 Script executed:
# Check what "redis" is in the Cargo.toml (it's in dist features but not declared as feature)
echo "=== Searching for redis references ==="
rg -n 'redis' Cargo.tomlRepository: txpipe/oura
Length of output: 235
🏁 Script executed:
# Check if the basic sinks are actually feature-gated in the code
echo "=== Checking for cfg_attr or feature guards on sinks ==="
rg -n 'feature.*=.*"(stdout|terminal|webhook)"' src/sinks/Repository: txpipe/oura
Length of output: 112
🏁 Script executed:
# Check if file_rotate has optional marker (it didn't in the deps)
echo "=== Looking for missing 'redis' feature flag definition ==="
rg -n 'redis' Cargo.tomlRepository: txpipe/oura
Length of output: 256
🏁 Script executed:
# Check what's in src/sinks/mod.rs to see if sinks are feature-gated
echo "=== Checking src/sinks/mod.rs for feature gates ==="
cat -n src/sinks/mod.rs | head -100Repository: txpipe/oura
Length of output: 2868
🏁 Script executed:
# Check src/sources/mod.rs for n2n/n2c
echo "=== Checking src/sources/mod.rs for n2n/n2c ==="
cat -n src/sources/mod.rs | head -100Repository: txpipe/oura
Length of output: 3099
🏁 Script executed:
# Search for redis feature definition that should exist
echo "=== Searching entire repo for redis feature definition ==="
rg -n 'redis\s*=' Cargo.tomlRepository: txpipe/oura
Length of output: 165
🏁 Script executed:
# Check the complete [features] section to ensure we have the full list
echo "=== Complete features section ==="
rg -A 30 '^\[features\]' Cargo.tomlRepository: txpipe/oura
Length of output: 1601
The documentation claim about always-on sources and sinks is accurate, but there's a missing feature definition for redis.
Verification confirms:
- n2n is unconditionally compiled (always-on) ✓
- n2c is always compiled on Unix targets (not feature-gated) ✓
- terminal, stdout, file_rotate, webhook are all unconditionally compiled (always-on) ✓
However, there's a critical mismatch: redis is listed in [package.metadata.dist] features but is missing a corresponding entry in the [features] section of Cargo.toml. It only has an optional = true dependency declaration. This discrepancy may cause cargo-dist build failures or unexpected behavior. Add redis = ["redis"] to the [features] section to match the dependency declaration and dist metadata configuration.
🤖 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 `@docs/v2/installation/binary_release.mdx` around lines 7 - 21, The
documentation accurately lists Redis as an included sink in the pre-built
binaries, but the Cargo.toml configuration doesn't have a corresponding feature
definition to support this claim. Add a `redis = ["redis"]` entry to the
`[features]` section of Cargo.toml to match the existing `optional = true`
dependency declaration and the redis entry in the `[package.metadata.dist]`
features configuration. This will ensure cargo-dist properly recognizes and
includes Redis in the build process.
Reduce Oura's dependency footprint so the released binary can bundle most integrations by default, instead of forcing consumers to compile custom feature-flavored builds. Base trims & dedup: - move reqwest to rustls and bump to 0.12, collapsing the duplicate native-tls/openssl and rustls/http/hyper stacks onto a single tree - gate handlebars behind the `sql` feature (its only user) - drop the unused `yaml` config format (keep toml + json) - bump bech32 0.9->0.11 and itertools 0.12->0.14 to dedupe with pallas - adopt gasket/gasket-prometheus 0.11.1, which drops prometheus_exporter_base and its legacy TLS/HTTP server stack (metrics endpoint verified working) Batteries-included release binary (~37 MB, fully pure-Rust TLS): - default dist features now bundle u5c, elasticsearch, hydra, redis, rabbitmq, sql, gcp and aws - keep sqlx, elasticsearch and google-cloud-pubsub on rustls so no system OpenSSL is pulled into the release build - leave kafka, zeromq, mithril and wasm opt-in (each needs a special build environment incompatible with portable cross-target releases) - wire the opt-in `kafka` feature to vendored OpenSSL (its only TLS backend is openssl-sys) so kafka / `--all-features` builds compile without a system OpenSSL; the release bundle stays pure-rustls - document the bundled vs. source-build integrations in the install docs Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
scarmuega
force-pushed
the
chore/slim-default-deps
branch
from
51530e5 to
23cfcd2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Oura's plugin architecture pulls in many heavy dependencies, so they're feature-flagged by group. That keeps the binary small but forces most consumers to compile their own custom flavor from source. This PR reduces the dependency footprint enough to ship a batteries-included default binary, while removing dead/duplicated dependencies along the way.
Base trims & dedup (no functionality loss)
openssldependency.reqwestto rustls and bump to 0.12, collapsing the duplicatenative-tls/opensslandrustls/http/hyperstacks onto a single tree.handlebarsbehind thesqlfeature (its only user).yamlconfig format (keeptoml+json).bech320.9→0.11 anditertools0.12→0.14 to dedupe withpallas.gasket/gasket-prometheus0.11.1, which dropsprometheus_exporter_baseand its legacy TLS/HTTP server stack (see gasket-prometheus: drop prometheus_exporter_base to stop pulling a legacy TLS/HTTP stack construkts/gasket-rs#37). The Prometheus metrics endpoint was verified working at runtime after the swap.Batteries-included release binary
The pre-built binaries now bundle every integration that builds cleanly across all release targets (~37 MB stripped, fully pure-Rust TLS — no system OpenSSL):
u5c,elasticsearch,hydra,redis,rabbitmq,sql,gcp,aws.sqlx,elasticsearch, andgoogle-cloud-pubsubare pinned to their rustls backends so no OpenSSL is pulled into the release build.Left opt-in (require a source build — each needs a special build environment incompatible with portable cross-target releases):
kafkazeromqlibzmqmithrilgmp-mpfr-sys), unsupported on Windows MSVCwasmThe install docs are updated to describe what ships by default vs. what needs a source build.
Verification
cargo testpasses (22/22), including the cip14 bech32 golden vectors.cargo tree -i openssl-sys/-i native-tlsare empty for the bundled feature set.:9186/metrics→ HTTP 200).🤖 Generated with Claude Code
Summary by CodeRabbit
sqlsupport.--all-features.