[chronik] Split indexer setup into prepare_paths and prepare_db
ClosedPublic
Actions

Authored by Fabien on Jul 30 2025, 09:37.

Details

Reviewers

tobias_ruck

Group Reviewers

Restricted Project

Commits

rABCca6bb0fd9b5e: [chronik] Split indexer setup into prepare_paths and prepare_db

Summary

This makes the code easier to read, but most important it makes the tests capable to execute without requiring a call to setup().
A follow up diff will add the node bridge to the indexer, requiring cxx which is not linked to the test profile (and I have no solution to achieve that as it's not supported by corrosion).

The db wiping unit test has been removed as it is already fully covered by the chronik_resync.py functional test.

Test Plan

ninja all check-crate-chronik

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

Fabien created this revision.Jul 30 2025, 09:37

Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 30 2025, 09:37

Fabien requested review of this revision.Jul 30 2025, 09:37

Fabien added a child revision: D18429: [chronik] Let the indexer access the node bridge.Jul 30 2025, 09:51

Harbormaster completed remote builds in B34002: Diff 55039.Jul 30 2025, 09:59

Not a very confident suggestion, not sure if we can create a helper function for let params = ChronikIndexerParams here? It seems like it could reduce a bit of code, but it doesn't make much difference.

In D18428#420057, @alitayin wrote:

Not a very confident suggestion, not sure if we can create a helper function for let params = ChronikIndexerParams here? It seems like it could reduce a bit of code, but it doesn't make much difference.

I don't see how it would reduce the code. Also outside of tests it only appears once.

tobias_ruck requested changes to this revision.Jul 31 2025, 10:21

tobias_ruck added a subscriber: tobias_ruck.

tobias_ruck added inline comments.

chronik/chronik-indexer/src/indexer.rs
274 ↗	(On Diff #55039)	this name is a bit misleading, as it doesn't really prepare a path (we would expect it to build us just a path (basically a string)) instead, it should indicate that it sets up the data directory, and wipes it if necessary, e.g. `prepare_db_folder`
274 ↗	(On Diff #55039)	A path is more like a string, but this has a side-effect on the disk so the name should reflect it. Consider making this `pub(crate)`, if it's only intended to be used for unittests
297 ↗	(On Diff #55039)	Consider making this `pub(crate)`, if it's only intended to be used for unittests