Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Index Provider integration #313

Closed
wants to merge 20 commits into from

Conversation

aarshkshah1992
Copy link
Collaborator

@aarshkshah1992 aarshkshah1992 commented Mar 8, 2022

Should be able to announce indices and serve indices for Boost deals and legacy markets deals.
Markets PR at filecoin-project/go-fil-markets#691.

TODO

  • Boost should start after markets so the index provider callback isn't overwritten.
  • Test Market deals announcement on Sofia Miner.
  • Test Boost deal announcement on Sofia Miner.
  • Test the AnnounceAllDeals CLI on Sofia Miner.

@aarshkshah1992 aarshkshah1992 changed the base branch from main to nonsense/legacy-cli-commands March 8, 2022 10:47
@aarshkshah1992 aarshkshah1992 changed the title Index Provider integration Index Provider integration and support for dagstore reinit Mar 8, 2022
@aarshkshah1992 aarshkshah1992 changed the title Index Provider integration and support for dagstore reinit Index Provider integration Mar 8, 2022
indexprovider/wrapper.go Outdated Show resolved Hide resolved
cmd/boost/index.go Outdated Show resolved Hide resolved
}
}

func (w *Wrapper) IndexerAnnounceAllDeals(ctx context.Context) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense for this method to announce all Boost and legacy markets deals?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can do it or we can do it as we're currently doing it. I don't have a strong preference.

Copy link
Contributor

Choose a reason for hiding this comment

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

Where possible I'd prefer to keep knowledge of the two different data sources inside the wrapper, rather than leaking it into the CLI layer

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Man, moving all that markets code here (which has intimate knowledge of various market states and what they mean) will add mantainence burden here and is something I'd like to avoid if possible. Remember, the plan is to eventually deprecate markets.

node/builder.go Outdated Show resolved Hide resolved
node/config/def.go Outdated Show resolved Hide resolved
node/modules/storageminer.go Show resolved Hide resolved
storagemarket/deal_execution.go Outdated Show resolved Hide resolved
storagemarket/provider.go Outdated Show resolved Hide resolved
storagemarket/types/deal_state.go Outdated Show resolved Hide resolved
@nonsense nonsense changed the base branch from nonsense/legacy-cli-commands to main March 9, 2022 09:43
@aarshkshah1992
Copy link
Collaborator Author

Subsumbed by #319

@nonsense nonsense deleted the feat/announce-indices-for-deals branch March 22, 2022 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants