Skip to content

Conversation

@Snawoot
Copy link
Contributor

@Snawoot Snawoot commented Aug 10, 2021

This PR implements external UDP port check routine.

Not integrating routine into node in this PR for two reasons:

  • Keep size reviewable.
  • @soffokl is now already working on changes in P2PListener which I'm about to touch too.

@Snawoot Snawoot marked this pull request as ready for review August 10, 2021 09:50

for _, address := range echoServerAddresses {
go func(echoServerAddress string) {
err := func() error {
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like a perfect candidate to extract to a separate function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied

responseChan := make(chan struct{})

// Background context-aware receiver
go func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Id also extract this as a separate func, can assign a comprehendable name to it then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied

@codecov-commenter
Copy link

codecov-commenter commented Aug 10, 2021

Codecov Report

Merging #3753 (62fbe87) into master (6eaa0c4) will decrease coverage by 0.13%.
The diff coverage is 0.00%.

❗ Current head 62fbe87 differs from pull request most recent head 955e642. Consider uploading reports for the commit 955e642 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3753      +/-   ##
==========================================
- Coverage   43.99%   43.86%   -0.14%     
==========================================
  Files         323      324       +1     
  Lines       16525    16588      +63     
==========================================
+ Hits         7270     7276       +6     
- Misses       8503     8559      +56     
- Partials      752      753       +1     
Impacted Files Coverage Δ
core/port/reachability.go 0.00% <0.00%> (ø)
nat/traversal/pinger.go 73.33% <0.00%> (-0.96%) ⬇️
session/pingpong/consumer_balance_tracker.go 57.51% <0.00%> (+0.86%) ⬆️
requests/dialer_swarm.go 77.09% <0.00%> (+3.81%) ⬆️

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 6eaa0c4...955e642. Read the comment docs.

Co-authored-by: Viktoras <vkuznecovas@users.noreply.github.com>
@Snawoot Snawoot merged commit d54e4b1 into master Aug 10, 2021
@Snawoot Snawoot deleted the port_check branch August 10, 2021 12:58
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.

5 participants