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

Allow custom configuration of transports #57

Merged
merged 5 commits into from
Jul 27, 2020

Conversation

hannahhoward
Copy link
Collaborator

Goals

Support the end goal of loading/storing storage/retrieval deals from custom block stores

Implementation

This is a bit fun/funky, but there are two basic adds:

  1. A general purpose system for hooking in on a given voucher type to allow configuration of the the transport medium. (This is a bit of an abstraction breaker, cause the configurer receives a generic transport interface, that they cast to the transport medium they want to configure -- though I think this is ok, because once we have multiple transport options, the configurer may want to take different actions for different transports -- it feels silly to try to build a generic interface for configuring transports -- the things you do with each will be different)

Anyway this is done with by registering a transport configurer which has this signature:

type TransportConfigurer func(datatransfer.ChannelID, datatransfer.Voucher, datatransfer.transport)

(oy Configurer... that's a word -- maybe there is something better?)

  1. A method for customizing the loader/storer for a given request on the graphsync transport medium. This uses graphsync's alternate persistence options under the hood.

You can see how this all comes together in the TestRoundTrip in integration test.

I also did some rearranging of files to avoid circular imports and break up the types file.

add a method to allow swapping of an alternate store for a given request
move interfaces to the top level package to avoid circular imports
Support a per voucher type transport configurer to allow custom configuration of a transport
@hannahhoward hannahhoward requested a review from ingar July 24, 2020 02:49
@hannahhoward
Copy link
Collaborator Author

arg... I really didn't mean for this to seem like such a large PR -- the rearranging of files to avoid the circular imports really grew quick. I'd focus on graphsync transport changes, and the implementation of the registry of configuration functions

Copy link
Contributor

@ingar ingar left a comment

Choose a reason for hiding this comment

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

Looks good, types cleanup FTW. One question about the test configuration; I didn't look too closely.

rootCid := root.(cidlink.Link).Cid

var destDagService ipldformat.DAGService
if data.customSourceStore {
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be data.customTargetStore?

@hannahhoward hannahhoward merged commit 5bb7276 into master Jul 27, 2020
@rvagg rvagg deleted the feat/configure-loader-storer branch January 5, 2023 01:09
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.

2 participants