Skip to content
This repository was archived by the owner on Sep 26, 2018. It is now read-only.

Commit 8a26f01

Browse files
committed
Refactor deprecated Transport.CancelRequest method
- Use context.WithCancel to cancel the watcher http request - Refactor code to allow closing the request body in a sane way - Declare variables in the in the narrowest possible scope
1 parent 3583069 commit 8a26f01

2 files changed

Lines changed: 29 additions & 23 deletions

File tree

provider/sidecar.go

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package provider
22

33
import (
4+
"context"
45
"encoding/json"
56
"fmt"
67
"io"
@@ -186,36 +187,42 @@ func (provider *Sidecar) sidecarWatcher(stop chan bool, pool *safe.Pool) {
186187
}
187188

188189
func (provider *Sidecar) recycleConn(stop chan bool, pool *safe.Pool) {
189-
var err error
190-
var resp *http.Response
191-
var req *http.Request
192190
for {
193191
select {
194192
case <-stop:
195193
return
196194
default:
197-
//use refresh interval to occasionally reconnect to Sidecar in case the stream connection is lost
198-
req, err = http.NewRequest("GET", provider.Endpoint+"/watch?by_service=false", nil)
199-
if err != nil {
200-
log.Errorf("Error creating http request to Sidecar: %s, Error: %s", provider.Endpoint, err)
201-
continue
202-
}
203-
resp, err = watcherHTTPClient.Do(req)
204-
if err != nil {
205-
log.Errorf("Error connecting to Sidecar: %s, Error: %s", provider.Endpoint, err)
206-
time.Sleep(5 * time.Second)
207-
continue
208-
}
195+
// Wrap logic in an anonymous function because the defer statement has function scope
196+
func() {
197+
// Use refresh interval to occasionally reconnect to Sidecar in case the stream connection is lost
198+
req, err := http.NewRequest("GET", provider.Endpoint+"/watch?by_service=false", nil)
199+
if err != nil {
200+
log.Errorf("Error creating watch request for Sidecar instance '%s': %s", provider.Endpoint, err)
201+
time.Sleep(5 * time.Second)
202+
return
203+
}
209204

210-
safe.Go(func() { decodeStream(resp.Body, provider.callbackLoader) })
205+
ctx, cancel := context.WithCancel(context.Background())
206+
// Cancel the infinite timeout request automatically after we reset connTimer
207+
defer cancel()
208+
209+
req = req.WithContext(ctx)
210+
211+
resp, err := watcherHTTPClient.Do(req)
212+
if err != nil {
213+
log.Errorf("Error connecting to Sidecar instance '%s': %s", provider.Endpoint, err)
214+
time.Sleep(5 * time.Second)
215+
return
216+
}
217+
defer resp.Body.Close()
211218

212-
//wait on refresh connection timer. If this expires we haven't seen an update in a
213-
//while and should cancel the request, reset the time, and reconnect just in case
214-
<-provider.connTimer.C
215-
provider.connTimer.Reset(time.Duration(provider.RefreshConn))
219+
safe.Go(func() { decodeStream(resp.Body, provider.callbackLoader) })
216220

217-
//TODO: Deprecated method. Refactor this to use a context.
218-
watcherHTTPTransport.CancelRequest(req)
221+
// Wait on refresh connection timer. If this expires we haven't seen an update in a
222+
// while and should cancel the request, reset the time, and reconnect just in case
223+
<-provider.connTimer.C
224+
provider.connTimer.Reset(time.Duration(provider.RefreshConn))
225+
}()
219226
}
220227
}
221228
}

provider/sidecar_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,6 @@ func TestSidecar(t *testing.T) {
370370

371371
So(configMsg.Configuration.Backends, ShouldContainKey, "api")
372372
So(configMsg.Configuration.Backends["api"].Servers["another-aws-host_9000"].URL, ShouldEqual, "http://169.254.1.1:9000")
373-
374373
})
375374

376375
Reset(func() {

0 commit comments

Comments
 (0)