Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 43 minutes and 33 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughThe PR refactors configuration handling across Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 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 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 |
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@geyser-plugin/configs/plugin-config.example.toml`:
- Around line 13-15: The example config currently enables ksqlDB restore by
providing a non-running default url under the [ksql] block (ksql.url =
"http://localhost:8088"); change the example so restore stays disabled by
default — either comment out the entire [ksql] block or remove/blank the
ksql.url entry (or prefix it with a comment) so that the generated config from
make init-config does not contain a live ksql.url and plugin startup won't
attempt a restore.
In `@geyser-plugin/plugin-config.toml`:
- Line 1: The committed plugin-config.toml contains a platform-specific libpath
("libpath = \"target/release/libsolana_accountsdb_plugin_kafka.so\"") which
breaks non-Linux launches; remove this generated root-level file from the PR (or
replace it with a neutral example) and put the platform-specific sample under
configs/ (or update .gitignore to ignore generated plugin-config.toml). Ensure
the root plugin-config.toml either does not exist in the repo or contains a
platform-agnostic example value rather than the .so path so make init-config /
make launch behave correctly across macOS (.dylib) and Linux (.so).
In `@geyser-plugin/README.md`:
- Around line 37-39: The Quick Start README currently includes an enabled [ksql]
block (keys: url and table) which causes startup restore to require ksqlDB;
instead update the Quick Start to treat this as optional by commenting out or
removing the [ksql] block (the "[ksql]" header and its "url" and "table"
entries) or move it to an "Optional configuration" section with clear comment
that it must be omitted for minimal local runs, so plugin load won't attempt a
ksqlDB restore on first run.
In `@geyser-plugin/src/config.rs`:
- Around line 162-191: When validating ksql.url in the existing block
(referencing self.ksql.url), also validate the configured table name: if
self.restore_on_startup is true (or startup restore is enabled) and ksql.url is
present, trim self.ksql.table and return a
GeyserPluginError::ConfigFileReadError with an appropriate message (e.g.,
"invalid config field `ksql.table`: table must not be empty") when the trimmed
table is empty; use the same error construction style as the existing checks so
the validation occurs up front (refer to self.ksql.table and
restore_on_startup).
In `@geyser-plugin/src/plugin/mod.rs`:
- Around line 221-228: The code uses the raw config.ksql.url (possibly with
surrounding whitespace) when logging and calling KsqlPubkeyRestoreClient::new;
change it to use the validated/trimmed value by trimming the Option<&str> before
use (e.g., extract the Some(raw) from config.ksql.url.as_deref(), call
raw.trim() and assign to url, then use that trimmed url in the info! log and in
KsqlPubkeyRestoreClient::new). Reference symbols: config.ksql.url,
KsqlPubkeyRestoreClient::new, and the local variable url.
- Around line 95-99: The producer config currently sets "bootstrap.servers"
before applying entries from config.kafka.client, which allows a later client
override; change the order so you first apply all entries from
config.kafka.client to producer_config (using producer_config.set for each (key,
value)), and only after that call producer_config.set("bootstrap.servers",
&config.kafka.bootstrap_servers) to ensure the typed bootstrap.servers value
(from config.kafka.bootstrap_servers) wins; update the code around
ClientConfig::new(), the loop over config.kafka.client, and the subsequent set
call accordingly.
In `@grpc-service/configs/config.toml`:
- Around line 1-17: Remove the tracked local default config file (config.toml)
so the example remains the single source of truth: delete config.toml from the
repository, add its filename to .gitignore, and commit that change; ensure the
init task (make init-config) still creates config.toml from config.example.toml
when absent. Reference the two files config.toml and config.example.toml and the
repository's init flow (make init-config) when making the change so local
operator-specific edits no longer show as tracked modifications.
In `@grpc-service/Makefile`:
- Line 11: The Makefile's run target is dropping the PUBKEY argument when
invoking the service; update the run target (and any other targets that invoke
the binary) to forward PUBKEY to the executed command by adding PUBKEY=$(PUBKEY)
(or exporting PUBKEY) in the command invocation so the binary still receives the
optional [PUBKEY] argument; ensure the run target's invocation mirrors how
CONFIG is forwarded so PUBKEY is preserved.
In `@grpc-service/src/kafka.rs`:
- Around line 31-39: The client map is being applied after the typed fields so
user-supplied keys (like "bootstrap.servers", "group.id", "auto.offset.reset")
can override canonical settings; change the order so that the raw client
settings from self.config.client are applied first to client_config and then
explicitly set the canonical fields (bootstrap.servers, group.id,
auto.offset.reset, enable.auto.commit) to guarantee they are authoritative, or
alternatively filter/reject those reserved keys from self.config.client before
applying; make the same change in every place you build a ClientConfig in this
file (the blocks around client_config creation and the later similar blocks).
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c07caaa8-a72b-4d02-a1ad-96cf1cf2757e
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
geyser-plugin/Cargo.tomlgeyser-plugin/Makefilegeyser-plugin/README.mdgeyser-plugin/configs/plugin-config.example.tomlgeyser-plugin/plugin-config.tomlgeyser-plugin/src/config.rsgeyser-plugin/src/plugin/mod.rsgrpc-service/Cargo.tomlgrpc-service/Makefilegrpc-service/README.mdgrpc-service/configs/config.example.tomlgrpc-service/configs/config.tomlgrpc-service/examples/grpc_client.rsgrpc-service/src/config.rsgrpc-service/src/errors.rsgrpc-service/src/grpc_service/dispatcher.rsgrpc-service/src/kafka.rsgrpc-service/src/ksql.rs
Amp-Thread-ID: https://ampcode.com/threads/T-019daf5d-d3be-775b-b6b2-dffd14139e26 Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019daf5d-d3be-775b-b6b2-dffd14139e26 Co-authored-by: Amp <amp@ampcode.com>
…rides Amp-Thread-ID: https://ampcode.com/threads/T-019daf5d-d3be-775b-b6b2-dffd14139e26 Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019daf5d-d3be-775b-b6b2-dffd14139e26 Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019daf5d-d3be-775b-b6b2-dffd14139e26 Co-authored-by: Amp <amp@ampcode.com>
Summary
Unify
grpc-serviceandgeyser-pluginconfiguration around TOML with aligned shared keys so the two configs can be translated with minimal edits.grpc-servicefrom env-driven startup config to TOML files with--config PATHsupportgeyser-pluginfrom JSON config to TOML while preserving raw Kafka client passthrough via[kafka.client]Details
This change standardizes the operator-facing config shape for the two Kafka-coupled crates in this workspace.
Shared values now use the same names and sections in both crates, centered on:
[kafka]forbootstrap_serversandtopic[ksql]forurlandtableThe plugin keeps low-level producer tuning available through
[kafka.client], whilegrpc-servicekeeps a typed consumer config and adds the same optional extension point for raw client overrides if needed later.grpc-service
cargo run -- [--config PATH] [PUBKEY]support without removing the existing optional pubkey filter behaviorgrpc-service/configs/geyser-plugin
[kafka]keys and startup restore settings to[ksql][plugin]rdkafkaclient passthrough through[kafka.client]Summary by CodeRabbit
New Features
--configcommand-line flag to specify custom configuration file pathsDocumentation
Chores