-
Notifications
You must be signed in to change notification settings - Fork 196
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
feat: Add option to pass external contexts into Go SDK's HTTP calls #1475
base: main
Are you sure you want to change the base?
Changes from 4 commits
cbe955e
d348f80
8fcc052
1154ed8
673a3ea
81ac004
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -118,3 +118,57 @@ func main() { | |
} | ||
} | ||
``` | ||
### Custom Context | ||
|
||
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. | ||
|
||
If you wish to include timeout functionality in your custom context then you should leverage [context.WithTimeout](https://pkg.go.dev/context#WithTimeout). | ||
|
||
> Note: Setting a context will override any "Timeout" settings that you have applied! | ||
|
||
#### Custom Context for all requests | ||
|
||
Follow the example code snippet below if you want all requests to use the same context: | ||
JCPistell marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
```go | ||
import "context" | ||
|
||
func main() { | ||
// sets a timeout of 5 minutes | ||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) | ||
defer cancel() | ||
|
||
cfg, err := rtl.NewSettingsFromFile("path/to/looker.ini", nil) | ||
cfg.Context = ctx | ||
|
||
session := rtl.NewAuthSession(cfg) | ||
sdk := v4.NewLookerSDK(session) | ||
} | ||
``` | ||
|
||
> 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. | ||
|
||
#### Custom Context per request | ||
|
||
Follow the example here to set a context for a specific request. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also add "For the specific request, the custom context will replace the custom context set for all requests in the previous "Custom Context for all requests" section." |
||
|
||
> 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! | ||
|
||
```go | ||
import "context" | ||
|
||
func main() { | ||
// sets a timeout of 5 minutes | ||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) | ||
defer cancel() | ||
|
||
cfg, err := rtl.NewSettingsFromFile("path/to/looker.ini", nil) | ||
session := rtl.NewAuthSession(cfg) | ||
sdk := v4.NewLookerSDK(session) | ||
|
||
sdk.Me("", &ApiSettings{Context: ctx}) | ||
} | ||
``` | ||
|
||
> 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also add "If you set a custom context for a specific request, the SDK will ignore any timeout described in the previous Timeout section for the specific request. You must set the timeout with the custom context for the specific request." There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't true - request timeouts will still apply - any specified context simply becomes the parent of the timeout request. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replace with
|
||
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my previous comment - this is inaccurate.