Allow custom decisioning for a provider to decide retrieval deals. #269
Conversation
Codecov Report
@@ Coverage Diff @@
## master #269 +/- ##
==========================================
+ Coverage 64.44% 65.20% +0.77%
==========================================
Files 40 40
Lines 2289 2362 +73
==========================================
+ Hits 1475 1540 +65
- Misses 694 698 +4
- Partials 120 124 +4
Continue to review full report at Codecov.
|
| // Stop stops handling incoming requests | ||
| func (p *provider) Stop() error { | ||
| func (p *Provider) Stop() error { |
There was a problem hiding this comment.
this had to be exported similarly to storagemarket, so a decider func is possible
shannonwells
changed the title
| @@ -189,6 +193,16 @@ func TestClientCanMakeDealWithProvider(t *testing.T) { | |||
| paramsV1: true, | |||
| selector: partialSelector, | |||
| unsealing: false}, | |||
| {name: "succeeds when using a custom decider function", | |||
| decider: func(ctx context.Context, state retrievalmarket.ProviderDealState) (bool, string, error) { | |||
There was a problem hiding this comment.
This quick and dirty integration test case is to minimize rebase conflicts for me, and will be properly set up in the PR for retrieval restart. See TODO comment below.
| @@ -36,21 +38,33 @@ func ReceiveDeal(ctx fsm.Context, environment ProviderDealEnvironment, deal rm.P | |||
| } | |||
|
|
|||
| // check that the deal parameters match our required parameters (or reject) | |||
| err = environment.CheckDealParams(dealProposal.PricePerByte, dealProposal.PaymentInterval, dealProposal.PaymentIntervalIncrease) | |||
| err = environment.CheckDealParams(dealProposal.PricePerByte, | |||
There was a problem hiding this comment.
I was conflicted here between moving all decisioning to DecideOnDeal and leaving just the param check here. I decided an up front parameter check is okay here before the "meat" of decisioning happens, but I can be convinced otherwise.
| @@ -119,22 +120,6 @@ func TestReceiveDeal(t *testing.T) { | |||
| require.NotEmpty(t, dealState.Message) | |||
| }) | |||
|
|
|||
| t.Run("response write error", func(t *testing.T) { | |||
There was a problem hiding this comment.
write responses are moved down to the decisioning function; this test is moved there as well.
| @@ -341,109 +326,113 @@ func TestProcessPayment(t *testing.T) { | |||
| }) | |||
| } | |||
|
|
|||
| type readBlockResponse struct { | |||
There was a problem hiding this comment.
this and below ere exported to their own spot so they can be reused in related tests
| type RetrievalProviderOption func(p *Provider) | ||
| type DealDecider func(ctx context.Context, state retrievalmarket.ProviderDealState) (bool, string, error) | ||
|
|
||
| type Provider struct { |
There was a problem hiding this comment.
Why did you decide to make this a public struct? I just want to make sure it's neccessary.
There was a problem hiding this comment.
It's similar to StorageMarket -- if you make it private you can't operate on (p Provider) to set a config option. StorageProviderOption and RetrievalProviderOption both take a (Storage|Retrieval)Provider.
Closes #268
NewProviderfunc a la storagemarketDealDeciderOptto generate aDeciderOptfunc with the providedDealDeciderfunc.