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

add the option to run with a context to control timeouts #2

Merged
merged 4 commits into from
Aug 9, 2021

Conversation

rdubrock
Copy link
Collaborator

@rdubrock rdubrock commented Aug 3, 2021

This PR does the following:

  • Adds a RunWithContext function
  • Modifies the Run and RunWithContext methods to return an error
  • Returns a timeout error if the context is cancelled or the maxUnknownHops value is reached

@rdubrock rdubrock requested review from mem and jewbetcha August 3, 2021 20:02
@@ -1,13 +1,16 @@
module github.com/tonobo/mtr

go 1.16
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

running go mod tidy did this, I'm not sure if that's what we want or what

case <-time.After(m.interval):
unknownHopsCount := 0

ttlLoop:
Copy link
Collaborator Author

@rdubrock rdubrock Aug 4, 2021

Choose a reason for hiding this comment

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

I'm not sure what the opinions are on loop labels. We need to break the loop from inside the select, and this seemed the most convenient way to do it, but I'm not sure if that's considered a code smell or not.

@rdubrock rdubrock merged commit 89d6b88 into master Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant