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

Fast retrieval support #305

Merged
merged 4 commits into from
Jul 1, 2020
Merged

Fast retrieval support #305

merged 4 commits into from
Jul 1, 2020

Conversation

ingar
Copy link
Contributor

@ingar ingar commented Jun 30, 2020

Summary

Send the fast retrieval flag along with the proposal and track it in the ClientDeal and MinerDeal. Also return this flag in the deal status query.

Resolves #285

@codecov-commenter
Copy link

codecov-commenter commented Jun 30, 2020

Codecov Report

Merging #305 into master will decrease coverage by 0.02%.
The diff coverage is 44.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #305      +/-   ##
==========================================
- Coverage   63.32%   63.31%   -0.01%     
==========================================
  Files          41       41              
  Lines        2538     2545       +7     
==========================================
+ Hits         1607     1611       +4     
- Misses        810      813       +3     
  Partials      121      121              
Impacted Files Coverage Δ
storagemarket/impl/client.go 3.26% <0.00%> (-0.01%) ⬇️
storagemarket/impl/provider.go 4.43% <0.00%> (-0.03%) ⬇️
storagemarket/types.go 0.00% <ø> (ø)
storagemarket/impl/clientstates/client_states.go 89.92% <100.00%> (+0.27%) ⬆️
...oragemarket/impl/providerstates/provider_states.go 86.12% <100.00%> (+0.08%) ⬆️
storagemarket/network/libp2p_impl.go 77.97% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7fe44f...120a966. Read the comment docs.

@ingar ingar requested a review from hannahhoward July 1, 2020 00:08
@ingar ingar marked this pull request as ready for review July 1, 2020 00:08
Copy link
Collaborator

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

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

Missing a component of the ticket:

Augment StorageProviderNode's OnDealComplete to take fast retrieval boolean

Ideally integration test should verify this bool matches when OnDealComplete is called on the testnode

@hannahhoward
Copy link
Collaborator

Otherwise LGTM

@ingar ingar requested a review from hannahhoward July 1, 2020 05:08
@ingar
Copy link
Contributor Author

ingar commented Jul 1, 2020

Note about changing the StorageProviderNode interface. That method gets passed a MinerDeal, which now has a FastRetrieval bool in it. I'm passing the FastRetrieval flag in via this existing parameter; it will avoid interface churn, unless we need the interface change for some other reason.

@ingar ingar force-pushed the feat/support-fast-retrieval branch from 38d6037 to 120a966 Compare July 1, 2020 17:06
@ingar ingar merged commit 525721f into master Jul 1, 2020
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.

StorageClient can request a fast retrieval
3 participants