Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Don't error when Snapshot is aborted #9492

Merged
merged 3 commits into from
Sep 10, 2018
Merged

Don't error when Snapshot is aborted #9492

merged 3 commits into from
Sep 10, 2018

Conversation

ngotchac
Copy link
Contributor

@ngotchac ngotchac commented Sep 7, 2018

This would previously cause to delete all downloaded chunks if the snapshot was aborted while restoring a chunk.

@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Sep 7, 2018
Copy link
Collaborator

@ordian ordian left a comment

Choose a reason for hiding this comment

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

LGTM

self.feed_chunk_with_restoration(&mut restoration, hash, chunk, is_state)
match self.feed_chunk_with_restoration(&mut restoration, hash, chunk, is_state) {
Ok(()) => (),
Err(Error(SnapshotErrorKind::Snapshot(SnapshotError::RestorationAborted), _)) => (),
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could unify this cases with

Ok(()) | 
Err(Error(<snip>)) => (),

@ordian ordian added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Sep 7, 2018
Copy link
Collaborator

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -584,10 +585,19 @@ impl Service {
}

/// Feed a chunk of either kind. no-op if no restoration or status is wrong.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe update the doc-string to "Feed a chunk of either Block or Status kind. No-op if … …"?

Copy link
Collaborator

@sorpaas sorpaas left a comment

Choose a reason for hiding this comment

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

LGTM!

@sorpaas sorpaas merged commit f3aed42 into master Sep 10, 2018
@sorpaas sorpaas deleted the ng-fix-snapshot-resume branch September 10, 2018 13:27
@5chdn 5chdn added this to the 2.2 milestone Sep 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants