Skip to content

[Persistence] Adds TreeStore Module#851

Merged
dylanlott merged 1 commit intomainfrom
tree-store-module
Jun 22, 2023
Merged

[Persistence] Adds TreeStore Module#851
dylanlott merged 1 commit intomainfrom
tree-store-module

Conversation

@dylanlott
Copy link
Contributor

@dylanlott dylanlott commented Jun 22, 2023

Description

Loads the TreeStore from a proper module and allows it access to the bus.

Issue

Refactor related to #756
This PR is the sum of feedback from #808.

Type of change

Please mark the relevant option(s):

  • New feature, functionality or library
  • Bug fix
  • Code health or cleanup
  • Major breaking change
  • Documentation
  • Other

List of changes

  • Makes the TreeStore component into a proper module.
  • Removes the txIndexer from the Update method interface.

Testing

  • make develop_test; if any code changes were made
  • make test_e2e on k8s LocalNet; if any code changes were made
  • e2e-devnet-test passes tests on DevNet; if any code was changed
  • Docker Compose LocalNet; if any major functionality was changed or introduced
  • k8s LocalNet; if any infrastructure or configuration changes were made

Required Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added, or updated, godoc format comments on touched members (see: tip.golang.org/doc/comment)
  • I have tested my changes using the available tooling
  • I have updated the corresponding CHANGELOG

If Applicable Checklist

  • I have updated the corresponding README(s); local and/or global
  • I have added tests that prove my fix is effective or that my feature works
  • I have added, or updated, mermaid.js diagrams in the corresponding README(s)
  • I have added, or updated, documentation and mermaid.js diagrams in shared/docs/* if I updated shared/*README(s)

@dylanlott dylanlott added persistence Persistence specific changes code health Nice to have code improvement e2e-devnet-test Runs E2E tests on devnet labels Jun 22, 2023
@dylanlott dylanlott self-assigned this Jun 22, 2023
@reviewpad reviewpad bot added the medium Pull request is medium label Jun 22, 2023
@dylanlott dylanlott force-pushed the tree-store-module branch from 07068f4 to 5b2f60b Compare June 22, 2023 16:24
@dylanlott dylanlott requested review from Olshansk and h5law June 22, 2023 16:25
@dylanlott dylanlott marked this pull request as ready for review June 22, 2023 16:26
Copy link
Contributor

@h5law h5law left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, straight port of the previous one so lets merge it in to main!

@dylanlott dylanlott merged commit 3d69a31 into main Jun 22, 2023
option(m)
}

m.SetBus(bus)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be registerModule or just setBus?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Olshansk I'll find out more about this tomorrow with @bryanchriswhite because I need more context from him to properly answer that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll rephrase, it should be RegisterModule IMO.

func (m *bus) RegisterModule(module modules.Module) {
	module.SetBus(m)
	m.modulesRegistry.RegisterModule(module)
}

SetBus: Get access to DI
RegisterModule: Get access to DI and make this module accessible via DI (e.g. IBC module getting access to trees module)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Get access to DI and make this module accessible via DI (e.g. IBC module getting access to trees module)

Yeah I added the GetTreeStore method back to persistence module but ideally now its a sub module we want to access it straight from the bus

bryanchriswhite added a commit that referenced this pull request Jun 23, 2023
* pokt/main:
  [Persistence] Update KVStore to use Badger wrapper functions (#838)
  [Persistence] Adds TreeStore Module (#851)
bryanchriswhite added a commit that referenced this pull request Jun 23, 2023
* refactor/unicast-router:
  [Persistence] Update KVStore to use Badger wrapper functions (#838)
  fix: gofmt
  [Persistence] Adds TreeStore Module (#851)
bryanchriswhite added a commit that referenced this pull request Jun 23, 2023
* feat/integrate-bg-router:
  [Persistence] Update KVStore to use Badger wrapper functions (#838)
  fix: gofmt
  [Persistence] Adds TreeStore Module (#851)
bryanchriswhite added a commit that referenced this pull request Jun 23, 2023
* feat/integrate-bg-router:
  [Persistence] Update KVStore to use Badger wrapper functions (#838)
  fix: gofmt
  [Persistence] Adds TreeStore Module (#851)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code health Nice to have code improvement e2e-devnet-test Runs E2E tests on devnet medium Pull request is medium persistence Persistence specific changes waiting-for-review

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants