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 Swarm<T>.Preload() #839

Merged
merged 4 commits into from
Apr 3, 2020
Merged

Conversation

longfin
Copy link
Member

@longfin longfin commented Apr 1, 2020

This PR addresses two problems of Swarm<T>.Preload().

  • Fixed a bug where Swarm<T> hadn't reported preload states correctly.
  • Fixed a Swarm<T>.PreloadAsync() method's bug that it had hung forever when a block fetch failed due to an unexpected error.

@longfin longfin self-assigned this Apr 1, 2020
@longfin longfin changed the title [WIP] Fix Swarm<T>.Preload() Fix Swarm<T>.Preload() Apr 2, 2020
@longfin longfin requested review from dahlia, moreal and earlbread and removed request for dahlia April 2, 2020 07:54
@longfin longfin added bug Something isn't working network Related to networking (Libplanet.Net) labels Apr 2, 2020
@longfin longfin marked this pull request as ready for review April 2, 2020 07:58
@codecov
Copy link

codecov bot commented Apr 2, 2020

Codecov Report

Merging #839 into master will increase coverage by 0.01%.
The diff coverage is 95.27%.

@@            Coverage Diff             @@
##           master     #839      +/-   ##
==========================================
+ Coverage   87.47%   87.48%   +0.01%     
==========================================
  Files         249      249              
  Lines       22520    22570      +50     
==========================================
+ Hits        19699    19746      +47     
- Misses       1475     1480       +5     
+ Partials     1346     1344       -2     
Impacted Files Coverage Δ
Libplanet/Net/BlockCompletion.cs 92.15% <93.91%> (-1.13%) ⬇️
Libplanet.Tests/Net/BlockCompletionTest.cs 98.34% <100.00%> (-0.17%) ⬇️
Libplanet/Store/DefaultStore.cs 85.44% <0.00%> (-0.95%) ⬇️
Libplanet/Crypto/PrivateKey.cs 85.34% <0.00%> (-0.87%) ⬇️
Libplanet/Net/Protocols/KademliaProtocol.cs 64.63% <0.00%> (-0.39%) ⬇️
Libplanet/Net/Swarm.cs 85.12% <0.00%> (+0.39%) ⬆️
Libplanet/Net/NetMQTransport.cs 84.06% <0.00%> (+0.80%) ⬆️

earlbread
earlbread previously approved these changes Apr 2, 2020
Libplanet/Net/BlockCompletion.cs Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
@longfin longfin requested review from dahlia and earlbread April 2, 2020 13:14
earlbread
earlbread previously approved these changes Apr 2, 2020
@longfin
Copy link
Member Author

longfin commented Apr 2, 2020

/rebase

dahlia
dahlia previously approved these changes Apr 2, 2020
@dahlia dahlia requested a review from earlbread April 2, 2020 13:40
@longfin longfin force-pushed the bugfix/ibd-report branch 2 times, most recently from 539b901 to c3ae3f0 Compare April 2, 2020 15:36
@longfin
Copy link
Member Author

longfin commented Apr 2, 2020

I've amended 01994c4 to clean up the internal queue properly.

@longfin longfin force-pushed the bugfix/ibd-report branch 2 times, most recently from 4cd1abf to fbc1d55 Compare April 3, 2020 04:52
@longfin longfin requested a review from dahlia April 3, 2020 05:49
@longfin
Copy link
Member Author

longfin commented Apr 3, 2020

I've amended 0f689a9 again and it seems to pass all tests.

@dahlia @moreal @earlbread Please review once more.

earlbread
earlbread previously approved these changes Apr 3, 2020
moreal
moreal previously approved these changes Apr 3, 2020
dahlia
dahlia previously approved these changes Apr 3, 2020
Libplanet/Net/BlockCompletion.cs Outdated Show resolved Hide resolved
Libplanet/Net/BlockCompletion.cs Outdated Show resolved Hide resolved
longfin added a commit to longfin/libplanet.net that referenced this pull request Apr 3, 2020
@longfin longfin dismissed stale reviews from dahlia, moreal, and earlbread via e44d443 April 3, 2020 06:21
earlbread
earlbread previously approved these changes Apr 3, 2020
@longfin
Copy link
Member Author

longfin commented Apr 3, 2020

I've added e44d443 to apply suggestions.

dahlia
dahlia previously approved these changes Apr 3, 2020
moreal
moreal previously approved these changes Apr 3, 2020
@longfin longfin dismissed stale reviews from moreal, dahlia, and earlbread via 2bf6f88 April 3, 2020 08:59
@longfin longfin merged commit 215b90d into planetarium:master Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working network Related to networking (Libplanet.Net)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants