refactor/test(ingest): more common ingester abstractions for pushing pool data #8475
Conversation
p0mvn
added
V:state/compatible/backport
WalkthroughThe updates encompass improvements to the pool-extraction and tracking mechanism within the application. Key changes include the introduction of new interfaces and structures for pool management, renaming several variables and functions for clarity, and transitioning to the new memory-based pool tracker. Testing functions were also modified to reflect these updates. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (7)
ingest/common/domain/pools.go (1)
1-1: Add method documentation forExtractAllandExtractChanged.The comments for the methods in the
PoolExtractorinterface should be more descriptive to clarify their purpose and usage.- // ExtractAll extracts all the pools available within the height associated - // with the context. + // ExtractAll extracts all the pools available within the block height + // associated with the context. This method is typically used for initial + // data ingestion or full data synchronization. - // ExtractChanged extracts the pools that were changed in the block height associated - // with the context. + // ExtractChanged extracts the pools that were changed in the block height + // associated with the context. This method is typically used for incremental + // data updates.ingest/common/domain/mocks/pools_extracter_mock.go (4)
1-1: Fix package import formatting.The import statements should be grouped and ordered for better readability.
import ( "github.com/cosmos/cosmos-sdk/types" commondomain "github.com/osmosis-labs/osmosis/v25/ingest/common/domain" )
9-9: Add comments for struct fields.The fields in the
PoolsExtractorMockstruct should have comments explaining their purpose.type PoolsExtractorMock struct { // AllBlockDataError is the error to return when ExtractAll is called. AllBlockDataError error // ChangedBlockDataError is the error to return when ExtractChanged is called. ChangedBlockDataError error // IsProcessAllBlockDataCalled is a flag indicating if ExtractAll was called. IsProcessAllBlockDataCalled bool // IsProcessAllChangedDataCalled is a flag indicating if ExtractChanged was called. IsProcessAllChangedDataCalled bool // ProcessAllBlockDataPanicMsg is the message to panic with when ExtractAll is called. ProcessAllBlockDataPanicMsg string // BlockPools is the data to return when ExtractAll or ExtractChanged is called. BlockPools commondomain.BlockPools }
26-26: Add method documentation forExtractAll.The
ExtractAllmethod should have a comment explaining its purpose and behavior.// ExtractAll implements commondomain.PoolExtractor. // It returns the BlockPools and any error set in the mock.
36-36: Add method documentation forExtractChanged.The
ExtractChangedmethod should have a comment explaining its purpose and behavior.// ExtractChanged implements commondomain.PoolExtractor. // It returns the BlockPools and any error set in the mock.ingest/common/domain/block_process_strategy_manager.go (1)
1-1: Add method documentation forBlockProcessStrategyManager.The interface methods should have comments explaining their purpose and behavior.
type BlockProcessStrategyManager interface { // ShouldPushAllData returns true if all data should be pushed. ShouldPushAllData() bool // MarkInitialDataIngested marks the initial data as ingested. // After calling this function, ShouldPushAllData should return false. MarkInitialDataIngested() // MarkErrorObserved marks that an error has been observed. // After calling this function, ShouldPushAllData should return true. MarkErrorObserved() }ingest/common/domain/block_process_strategy_manager_test.go (1)
20-20: Fix typo in the comment.There is a typo in the comment. "Invariant" should be "invariant".
// Validates the invariant that the block process strategy manager.
Closes: #XXX
What is the purpose of the change
Separates the remaining
ingestabstractions to be used and shared betweenindexerandsqs.Namely,
BlockProcessStrategyManager- determines whether to push all blocks or only changedPoolExtracter- extracts pools - either all or only changedMoved
sqs_pool_tracker.gointomemory_pool_tracker.goSee #8470 for usage of the abstractions
Drive By
Renamed "extracter" to "extractor" due to linter