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

fix(Fetch Do): fetch all the blocks in the manifest, do not diff #4

Merged
merged 1 commit into from
Dec 12, 2018

Conversation

ramfox
Copy link
Member

@ramfox ramfox commented Dec 12, 2018

Right now, Missing() uses the nodegetter method Get
if you are online, this method attempts to the get the blocks off the network, not just attempting to get them locally. This takes a long time and makes Missing() unusable as of right now. Instead, we are passing NewCompletion an empty diff Manifest. We will be asking the remote source for the entire list of blocks, and although in certain cases this may be redundant, it is ultimately faster until we can change Missing

@ghost ghost assigned ramfox Dec 12, 2018
@ghost ghost added the in progress label Dec 12, 2018
@ramfox ramfox requested review from b5 and dustmop December 12, 2018 23:03
Copy link
Member

@b5 b5 left a comment

Choose a reason for hiding this comment

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

booooo, but yea ok let's merge

Copy link
Contributor

@dustmop dustmop left a comment

Choose a reason for hiding this comment

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

LGTM

@ramfox ramfox merged commit 9d4810b into master Dec 12, 2018
@ghost ghost removed the in progress label Dec 12, 2018
@ramfox ramfox deleted the missing branch December 12, 2018 23:08
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