Skip to content

Commit 8fcc052

Browse files
committed
updating tests and readme based on feedback
1 parent d348f80 commit 8fcc052

File tree

2 files changed

+114
-4
lines changed

2 files changed

+114
-4
lines changed

go/README.md

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,10 @@ func main() {
122122

123123
If you want even greater control over the lifecycle of the HTTP requests consider providing a [context](https://pkg.go.dev/context). Contexts can be set for all requests or per request.
124124

125+
If you wish to include timeout functionality in your custom context then you should leverage [context.WithTimeout](https://pkg.go.dev/context#WithTimeout).
126+
127+
> Note: Setting a context will override any "Timeout" settings that you have applied!
128+
125129
#### Custom Context for all requests
126130

127131
Follow the example code snippet below if you want all requests to use the same context:
@@ -131,7 +135,7 @@ import "context"
131135

132136
func main() {
133137
// sets a timeout of 5 minutes
134-
ctx, cancel := context.WithTimout(context.Background(), 5*time.Minute)
138+
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
135139
defer cancel()
136140

137141
cfg, err := rtl.NewSettingsFromFile("path/to/looker.ini", nil)
@@ -142,16 +146,20 @@ func main() {
142146
}
143147
```
144148

149+
> Note: A context set here will also apply to background requests to fetch/refresh oauth tokens, which are normally separated from contexts set via the Timeout property.
150+
145151
#### Custom Context per request
146152

147-
Follow the example here to set a context for a specific request. **This will override any context set in the SDK config as outlined in the previous section.**
153+
Follow the example here to set a context for a specific request.
154+
155+
> Note: This will override any "Timeout" settings that you have applied, as well as any context set in the SDK config as outlined in the previus section!
148156
149157
```go
150158
import "context"
151159

152160
func main() {
153161
// sets a timeout of 5 minutes
154-
ctx, cancel := context.WithTimout(context.Background(), 5*time.Minute)
162+
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
155163
defer cancel()
156164

157165
cfg, err := rtl.NewSettingsFromFile("path/to/looker.ini", nil)
@@ -160,4 +168,7 @@ func main() {
160168

161169
sdk.Me("", &ApiSettings{Context: ctx})
162170
}
163-
```
171+
```
172+
173+
> Note: Setting a context per request will NOT affect the context used for the background token fetching requests. If you have also set a context for all requests as mentioned above then that context
174+
will still be used for the token requests, otherwise the SDK will fall back on using a completely separate context for the token fetching requests.

go/rtl/auth_test.go

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -694,6 +694,105 @@ func TestAuthSession_Do_Timeout(t *testing.T) {
694694
t.Errorf("Do() call did not error with context.DeadlineExceeded, got=%v", err)
695695
}
696696
})
697+
698+
t.Run("Timeout set in Do()'s options overrides Authsession", func(t *testing.T) {
699+
mux := http.NewServeMux()
700+
setupApi40Login(mux, foreverValidTestToken, http.StatusOK)
701+
server := httptest.NewServer(mux)
702+
defer server.Close()
703+
704+
mux.HandleFunc("/api"+apiVersion+path, func(w http.ResponseWriter, r *http.Request) {
705+
time.Sleep(4 * time.Second)
706+
})
707+
708+
session := NewAuthSession(ApiSettings{
709+
BaseUrl: server.URL,
710+
ApiVersion: apiVersion,
711+
Timeout: 5,
712+
})
713+
714+
options := ApiSettings{
715+
Timeout: 1, //seconds
716+
}
717+
718+
err := session.Do(nil, "GET", apiVersion, path, nil, nil, &options)
719+
720+
if err == nil {
721+
t.Errorf("Do() call did not error/timeout")
722+
} else if !errors.Is(err, context.DeadlineExceeded) {
723+
t.Errorf("Do() call did not error with context.DeadlineExceeded, got=%v", err)
724+
}
725+
})
726+
727+
t.Run("Context set in AuthSession config overrides Timeouts", func(t *testing.T) {
728+
mux := http.NewServeMux()
729+
setupApi40Login(mux, foreverValidTestToken, http.StatusOK)
730+
server := httptest.NewServer(mux)
731+
defer server.Close()
732+
733+
mux.HandleFunc("/api"+apiVersion+path, func(w http.ResponseWriter, r *http.Request) {
734+
time.Sleep(4 * time.Second)
735+
})
736+
737+
ctx, cncl := context.WithTimeout(context.Background(), 1*time.Second)
738+
defer cncl()
739+
740+
session := NewAuthSession(ApiSettings{
741+
BaseUrl: server.URL,
742+
ApiVersion: apiVersion,
743+
Context: ctx,
744+
Timeout: 5,
745+
})
746+
747+
options := ApiSettings{
748+
Timeout: 5,
749+
}
750+
751+
err := session.Do(nil, "GET", apiVersion, path, nil, nil, &options)
752+
753+
if err == nil {
754+
t.Errorf("Do() call did not error/timeout")
755+
} else if !errors.Is(err, context.DeadlineExceeded) {
756+
t.Errorf("Do() call did not error with context.DeadlineExceeded, got=%v", err)
757+
}
758+
})
759+
760+
t.Run("Context set in options overrides config ctx and all Timeouts", func(t *testing.T) {
761+
mux := http.NewServeMux()
762+
setupApi40Login(mux, foreverValidTestToken, http.StatusOK)
763+
server := httptest.NewServer(mux)
764+
defer server.Close()
765+
766+
mux.HandleFunc("/api"+apiVersion+path, func(w http.ResponseWriter, r *http.Request) {
767+
time.Sleep(4 * time.Second)
768+
})
769+
770+
octx, ocncl := context.WithTimeout(context.Background(), 1*time.Second)
771+
defer ocncl()
772+
773+
sctx, scncl := context.WithTimeout(context.Background(), 5*time.Second)
774+
defer scncl()
775+
776+
session := NewAuthSession(ApiSettings{
777+
BaseUrl: server.URL,
778+
ApiVersion: apiVersion,
779+
Context: sctx,
780+
Timeout: 5,
781+
})
782+
783+
options := ApiSettings{
784+
Timeout: 5,
785+
Context: octx,
786+
}
787+
788+
err := session.Do(nil, "GET", apiVersion, path, nil, nil, &options)
789+
790+
if err == nil {
791+
t.Errorf("Do() call did not error/timeout")
792+
} else if !errors.Is(err, context.DeadlineExceeded) {
793+
t.Errorf("Do() call did not error with context.DeadlineExceeded, got=%v", err)
794+
}
795+
})
697796
}
698797

699798
func TestSetQuery(t *testing.T) {

0 commit comments

Comments
 (0)