handle N targets #2
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| failed := false | ||
| for i, v := range targets { | ||
| // Verify the target is HTTP or HTTPS. | ||
| if !strings.HasPrefix(v, "http://") && !strings.HasPrefix(v, "https://") { |
There was a problem hiding this comment.
i would rip this out into a separate function: ValidateTarget or something
|
now that #1 is merged, give this one a rebase and i'll CR/M |
Standard USAGE format Writes usage & general errors to stderr **Handles N targets** URL validation (url.Parse doesn't do validation) Generally more comments throughout Returns 504 Gateway Timeout when all targets fail (rather than 404 Not Found)
1. When 'failed' is closed !ok will be true -- we weren't properly hitting that case and the connection would stay open. 2. Cancels the request when a bad response code is hit.
hackergrrl
force-pushed
the
multipass
branch
from
2b00a11
to
dc4337a
Compare
|
Updated. Please take another look @whyrusleeping! |
| @@ -7,142 +7,150 @@ import ( | |||
| "net/http" | |||
| "net/url" | |||
| "os" | |||
| "reflect" | |||
There was a problem hiding this comment.
Things that make me say "here we go..." for $200 Alex.
| @@ -8,141 +8,131 @@ import ( | |||
| "net/url" | |||
| "os" | |||
| "strings" | |||
|
|
|||
There was a problem hiding this comment.
Yeah, reflection isn't necessary here. Let's just use a single channel.
| resp_b <- resp | ||
| // Wait for a successful response (or failures across the board). | ||
| for { | ||
| chosen, value, _ := reflect.Select(cases) |
There was a problem hiding this comment.
i really feel like you could get away without using reflection here. Have one channel that a goroutine can send its index on when its done or something, then can grab the response from an array by its index.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In addition to handling N targets, I also made a few other changes while digging around:
url.Parse()doesn't do validation)504 Gateway Timeoutwhen all targets fail (rather than404 Not Found)Handling N targets ended up being a significant refactor, so it was difficult to separate that change from the above bullet pointed items. I can try and pull them all apart into separate commits, but they're all small things and shouldn't be too hard to examine inline.