Clickhouse backup on rbuilder-utils #117
Conversation
thedevbirb
left a comment
thedevbirb
left a comment
There was a problem hiding this comment.
I think just moving some stuff around is all it's needed here, so comments are mostly about that.
src/indexer/click/mod.rs
Outdated
| /// The channel where to send data to be backed up. | ||
| backup_tx: mpsc::Sender<FailedCommit<T>>, | ||
| } | ||
| impl rbuilder_utils::clickhouse_with_backup::metrics::Metrics for Metrics { |
There was a problem hiding this comment.
I think this should be implemented directly on IndexerMetrics, given it is a namespace struct too.
There was a problem hiding this comment.
in the trait rbuilder_utils::clickhouse_with_backup::metrics::Metrics all the "clickhouse" on the methods was removed since it was redundant.
eg: Metrics::increment_write_failures gets mapped to IndexerMetrics::increment_clickhouse_write_failures.
src/indexer/click/backup.rs
Outdated
There was a problem hiding this comment.
I'd just move this tests in mod.rs then, since it's all there is left.
src/indexer/click/primitives.rs
Outdated
There was a problem hiding this comment.
I'd move the remaining implementations here inside models.rs.
|
Sorry, after repo transferring and some mandatory history rewrite this needs rebasing. |
ZanCorDX
force-pushed
the
clickhouse-backup-on-utils
branch
from
8d485c5 to
1995426
Compare
mempirate
requested review from
efbig,
mempirate,
metachris and
plroman
as code owners
thedevbirb
left a comment
thedevbirb
left a comment
There was a problem hiding this comment.
Two minor requests, and then good to go!
rust-toolchain.toml
Outdated
| channel = "stable" | ||
| version = "1.89.0" | ||
| channel = "nightly-2025-09-21" | ||
| components = ["rustfmt", "clippy"] |
There was a problem hiding this comment.
This doesn't specify a toolchain to use to build the code, and we don't want to use nightly for compiling. Right now, we want to use it only for the formatting, unless we find a good set of options available with stable only.
Moreover, my editor's LSP does not start because it can't pick up a version from this file.
I'd suggest to revert these changes.
There was a problem hiding this comment.
sorry this was not supposed to be comited!
Cargo.toml
Outdated
| #rbuilder-primitives = {path="./../rbuilder/crates/rbuilder-primitives"} | ||
| rbuilder-utils = { git = "https://github.com/flashbots/rbuilder", rev = "77f35b4ecd39ac547e92de2f11a7497676d3bfd4", features = ["test-utils"] } | ||
| #rbuilder-utils = {path="./../rbuilder/crates/rbuilder-utils"} |
There was a problem hiding this comment.
Can we remove these comments?
Created a new crate on rbuilder and moved the clickhouse back stuff and some other basic things there.
This PR should go after we merge flashbots/rbuilder#780