Fix mutierr appending; add a unit test. #33
Conversation
k8s-ci-robot
added
cncf-cla: yes
| go func() { | ||
| wg.Wait() | ||
| close(errCh) | ||
| }() |
There was a problem hiding this comment.
why is this waiting on the group in a separate goroutine?
There was a problem hiding this comment.
Added more details in the comment to explain this.
It needs to be unblocking so that the code below can wait for the errors to come back from the channel.
| if err != nil { | ||
| return nil, fmt.Errorf("failed to create request: %v", err) | ||
| } | ||
| resp, err := http.DefaultClient.Do(req) |
There was a problem hiding this comment.
what is the difference compared to the original code that used Get? the ability to pass context?
There was a problem hiding this comment.
Yes. I'd like to make sure the requests don't hang there which blocks the metrics refresh loop.
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahg-g, liu-cong The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
No description provided.