feat: Add Go SDK custom header support and README #1288

jeremytchang · 2023-03-29T23:55:39Z

Following our node sdk's design, allow developers to set custom headers on Go SDK requests. Request specific custom headers will override headers defined in the sdk's settings/config. This will allow the Go SDK caller to set Accept headers and other custom headers. Also add README for Go SDK.

Also fixes issue stated in #1075. (Not the main one, but other one brought up by Go SDK users)

jeremytchang · 2023-03-29T23:57:51Z

go/rtl/auth_test.go

@@ -125,70 +125,6 @@ func TestAuthSession_Do_Authorization(t *testing.T) {
 	})
 }

-func TestAuthSession_Do_UserAgent(t *testing.T) {


Moved these tests to header section.

github-actions · 2023-03-30T00:03:39Z

Go Tests

    6 files ±0     6 suites ±0 13m 52s ⏱️ + 1m 45s
  50 tests +1   48 ✔️ - 1 0 💤 ±0 2 ❌ +2
120 runs +2 118 ✔️ ±0 0 💤 ±0 2 ❌ +2

For more details on these failures, see this check.

Results for commit 295df8c. ± Comparison against base commit 44974ec.

This pull request removes 6 and adds 7 tests. Note that renamed tests count towards both.

rtl ‑ TestAuthSession_Do_Content_Type
rtl ‑ TestAuthSession_Do_Content_Type/Do()_sets_Content-Type_header_to_&#x27;application/json&#x27;_if_body_is_json
rtl ‑ TestAuthSession_Do_Content_Type/Do()_sets_Content-Type_header_to_&#x27;text/plain&#x27;_if_body_is_a_string
rtl ‑ TestAuthSession_Do_UserAgent
rtl ‑ TestAuthSession_Do_UserAgent/Do()_sets_User-Agent_header_with_AuthSession_config&#x27;s_AgentTag
rtl ‑ TestAuthSession_Do_UserAgent/Do()_sets_User-Agent_header_with_Do&#x27;s_option&#x27;s_AgentTag,_which_will_overwrite__AuthSession_config

rtl ‑ TestAuthSession_Do_Headers
rtl ‑ TestAuthSession_Do_Headers/Do()&#x27;s_options.Headers_will_overwrite_the_Headers_in_the_AuthSession&#x27;s_api_settings
rtl ‑ TestAuthSession_Do_Headers/Do()_sets_Content-Type_header_to_&#x27;application/json&#x27;_if_body_is_json
rtl ‑ TestAuthSession_Do_Headers/Do()_sets_Content-Type_header_to_&#x27;text/plain&#x27;_if_body_is_a_string
rtl ‑ TestAuthSession_Do_Headers/Do()_sets_User-Agent_header_with_AuthSession_config&#x27;s_AgentTag
rtl ‑ TestAuthSession_Do_Headers/Do()_sets_User-Agent_header_with_Do&#x27;s_option&#x27;s_AgentTag,_which_will_overwrite__AuthSession_config
rtl ‑ TestAuthSession_Do_Headers/Do()_sets_custom_headers_if_Headers_is_set_in_the_AuthSession&#x27;s_api_settings

github-actions · 2023-03-31T20:55:47Z

Go Tests

    6 files ±0     6 suites ±0 13m 16s ⏱️ + 1m 9s
  50 tests +1   48 ✔️ - 1 0 💤 ±0 2 ❌ +2
120 runs +2 118 ✔️ ±0 0 💤 ±0 2 ❌ +2

For more details on these failures, see this check.

Results for commit 295df8c. ± Comparison against base commit 44974ec.

This pull request removes 6 and adds 7 tests. Note that renamed tests count towards both.

rtl ‑ TestAuthSession_Do_Content_Type
rtl ‑ TestAuthSession_Do_Content_Type/Do()_sets_Content-Type_header_to_&#x27;application/json&#x27;_if_body_is_json
rtl ‑ TestAuthSession_Do_Content_Type/Do()_sets_Content-Type_header_to_&#x27;text/plain&#x27;_if_body_is_a_string
rtl ‑ TestAuthSession_Do_UserAgent
rtl ‑ TestAuthSession_Do_UserAgent/Do()_sets_User-Agent_header_with_AuthSession_config&#x27;s_AgentTag
rtl ‑ TestAuthSession_Do_UserAgent/Do()_sets_User-Agent_header_with_Do&#x27;s_option&#x27;s_AgentTag,_which_will_overwrite__AuthSession_config

rtl ‑ TestAuthSession_Do_Headers
rtl ‑ TestAuthSession_Do_Headers/Do()&#x27;s_options.Headers_will_overwrite_the_Headers_in_the_AuthSession&#x27;s_api_settings
rtl ‑ TestAuthSession_Do_Headers/Do()_sets_Content-Type_header_to_&#x27;application/json&#x27;_if_body_is_json
rtl ‑ TestAuthSession_Do_Headers/Do()_sets_Content-Type_header_to_&#x27;text/plain&#x27;_if_body_is_a_string
rtl ‑ TestAuthSession_Do_Headers/Do()_sets_User-Agent_header_with_AuthSession_config&#x27;s_AgentTag
rtl ‑ TestAuthSession_Do_Headers/Do()_sets_User-Agent_header_with_Do&#x27;s_option&#x27;s_AgentTag,_which_will_overwrite__AuthSession_config
rtl ‑ TestAuthSession_Do_Headers/Do()_sets_custom_headers_if_Headers_is_set_in_the_AuthSession&#x27;s_api_settings

github-actions · 2023-04-04T23:59:16Z

Go Tests

    6 files ±0     1 errors 5 suites - 1 13m 22s ⏱️ + 1m 15s
  50 tests +1   48 ✔️ -   1 0 💤 ±0 2 ❌ +2
110 runs - 8 108 ✔️ - 10 0 💤 ±0 2 ❌ +2

For more details on these parsing errors and failures, see this check.

Results for commit 36e8a02. ± Comparison against base commit 44974ec.

This pull request removes 6 and adds 7 tests. Note that renamed tests count towards both.

rtl ‑ TestAuthSession_Do_Content_Type
rtl ‑ TestAuthSession_Do_Content_Type/Do()_sets_Content-Type_header_to_&#x27;application/json&#x27;_if_body_is_json
rtl ‑ TestAuthSession_Do_Content_Type/Do()_sets_Content-Type_header_to_&#x27;text/plain&#x27;_if_body_is_a_string
rtl ‑ TestAuthSession_Do_UserAgent
rtl ‑ TestAuthSession_Do_UserAgent/Do()_sets_User-Agent_header_with_AuthSession_config&#x27;s_AgentTag
rtl ‑ TestAuthSession_Do_UserAgent/Do()_sets_User-Agent_header_with_Do&#x27;s_option&#x27;s_AgentTag,_which_will_overwrite__AuthSession_config

rtl ‑ TestAuthSession_Do_Headers
rtl ‑ TestAuthSession_Do_Headers/Do()&#x27;s_options.Headers_will_overwrite_the_Headers_in_the_AuthSession&#x27;s_api_settings
rtl ‑ TestAuthSession_Do_Headers/Do()_sets_Content-Type_header_to_&#x27;application/json&#x27;_if_body_is_json
rtl ‑ TestAuthSession_Do_Headers/Do()_sets_Content-Type_header_to_&#x27;text/plain&#x27;_if_body_is_a_string
rtl ‑ TestAuthSession_Do_Headers/Do()_sets_User-Agent_header_with_AuthSession_config&#x27;s_AgentTag
rtl ‑ TestAuthSession_Do_Headers/Do()_sets_User-Agent_header_with_Do&#x27;s_option&#x27;s_AgentTag,_which_will_overwrite__AuthSession_config
rtl ‑ TestAuthSession_Do_Headers/Do()_sets_custom_headers_if_Headers_is_set_in_the_AuthSession&#x27;s_api_settings

github-actions · 2023-04-05T00:49:15Z

Go Tests

    6 files     6 suites 12m 20s ⏱️
  50 tests   50 ✔️ 0 💤 0 ❌
120 runs 120 ✔️ 0 💤 0 ❌

Results for commit 0f60c79.

jkaster

This is going to be some appreciated new functionality

jkaster · 2023-04-05T00:55:43Z

go/rtl/auth.go

@@ -142,7 +142,7 @@ func (s *AuthSession) Do(result interface{}, method, ver, path string, reqPars m
 	}

 	// set headers
-	req.Header.Add("Content-Type", contentTypeHeader)
+	req.Header.Set("Content-Type", contentTypeHeader)

 	if s.Config.AgentTag != "" {
 		req.Header.Set("User-Agent", s.Config.AgentTag)


you should also set x-looker-appid like the other SDKs do. There are some request routers that will modify the User-Agent header. Currently this is set in the existing auth.go and the intent is that AgentTag overwrites that default header value.

Good callout, was unaware of the behavior of AgentTag in relation to x-looker-appid. Will have a separate ticket to tackle that.

jkaster · 2023-04-05T00:58:27Z

go/README.md

+func main() {
+    // Get settings from either looker.ini file OR environment:
+	// looker.ini file
+	cfg, err := rtl.NewSettingsFromFile("path/to/looker.ini", nil)
+    // environment
+    cfg, err := rtl.NewSettingsFromEnv()
+
+	// Create new auth session with sdk settings. 
+    // The auth session will fetch/refresh the access 
+    // token from your Looker instance's `login` endpoint. 
+    session := rtl.NewAuthSession(cfg)
+
+    // Create new instance of the Go Looker SDK 
+	sdk := v4.NewLookerSDK(session)
+
+    // Call the Looker API e.g. get your user's name
+    me, err := sdk.Me("", nil)
+	fmt.Printf("Your name is %s %s\n", *(me.first), *(me.last))
+}


code alignment seems strange here

jkaster · 2023-04-05T00:58:52Z

go/README.md

+
+You can set a custom timeout (in seconds) on Looker Go SDK's requests. The timeout defaults to 120 seconds. A timeout can either be applied to all outgoing requests or per outgoing request. 
+
+#### Timeout for all requests


great addition!

jkaster · 2023-04-05T00:59:13Z

go/README.md

+}
+```
+
+#### Custom headers per request


This will definitely be handy in some use cases

jkaster · 2023-04-05T00:59:40Z

go/README.md

+	cfg, err := rtl.NewSettingsFromFile("path/to/looker.ini", nil)
+    session := rtl.NewAuthSession(cfg)
+	sdk := v4.NewLookerSDK(session)
+
+    // Set the timeout in the options passed into the SDK method
+    me, err := sdk.Me("", &ApiSettings{Timeout: 60})
+
+    if errors.Is(err, context.DeadlineExceeded) {
+        // Timeout exceeded
+    }


strange indentation again? is it mixed tabs and spaces?

Yup mixed by accident. Fixed!

github-actions · 2023-04-05T01:02:17Z

Go Tests

    6 files     1 errors 5 suites 10m 31s ⏱️
  50 tests   48 ✔️ 0 💤 2 ❌
110 runs 108 ✔️ 0 💤 2 ❌

For more details on these parsing errors and failures, see this check.

Results for commit fd2887f.

github-actions · 2023-04-05T01:43:53Z

Go Tests

    6 files     6 suites 14m 57s ⏱️
  50 tests   48 ✔️ 0 💤 2 ❌
120 runs 118 ✔️ 0 💤 2 ❌

For more details on these failures, see this check.

Results for commit 513d940.

github-actions · 2023-04-05T01:51:13Z

Go Tests

    6 files     6 suites 14m 19s ⏱️
  50 tests   50 ✔️ 0 💤 0 ❌
120 runs 120 ✔️ 0 💤 0 ❌

Results for commit 513d940.

jkaster

LGTM with the follow-up ticket

Following our node sdk's design, allow developers to set custom headers on Go SDK requests. Request specific custom headers will override headers defined in the sdk's settings/config. This will allow the Go SDK caller to set Accept headers and other custom headers. Also add README for Go SDK. Also fixes issue stated in #1075. (Not the main one, but other one brought up by Go SDK users)

* Update state variables before moving to redux * Updating state variable names * fix isConfigured function * fix: Set Go SDK's User-Agent header (#1285) Implement AgentTag defined in settings and set Go SDK's outgoing Do requests with the User-Agent header. Fixes issue: #1274 --------- Co-authored-by: Colin Gooding <cgooding@vendasta.com> * fix: Implement Go SDK timeout (#1287) Implement the Timeout field in our config/settings. Make use of request context to control timeout. Timeout defaults to 120 seconds. Hopefully the unit tests are not flaky with the new timeout tests. Fixes issue #692 * feat: generate SDKs for Looker 23.4 (#1290) Release-As: 23.4.0 * chore: release main (#1281) * feat: Add Go SDK custom header support and README (#1288) Following our node sdk's design, allow developers to set custom headers on Go SDK requests. Request specific custom headers will override headers defined in the sdk's settings/config. This will allow the Go SDK caller to set Accept headers and other custom headers. Also add README for Go SDK. Also fixes issue stated in #1075. (Not the main one, but other one brought up by Go SDK users) * Updating OAuthForm function names --------- Co-authored-by: jeremytchang <78522362+jeremytchang@users.noreply.github.com> Co-authored-by: Colin Gooding <cgooding@vendasta.com> Co-authored-by: looker-open-source-automation-bot <looker-open-source-automation-bot@google.com>

jeremytchang added 2 commits March 29, 2023 17:07

wip

7633c46

wip

295df8c

jeremytchang commented Mar 29, 2023

View reviewed changes

jeremytchang mentioned this pull request Mar 30, 2023

[Go SDK] Request fails when the string fields of request body json contains percent sign #1075

Closed

wip

c538d38

jeremytchang changed the title ~~feat: Add Go SDK custom header support~~ feat: Add Go SDK custom header support and README Apr 4, 2023

wip

36e8a02

jeremytchang requested review from jkaster and removed request for jkaster April 4, 2023 23:55

Merge branch 'main' into customheadergo

0f60c79

fix readme typo

fd2887f

jeremytchang requested a review from jkaster April 5, 2023 00:59

jkaster requested changes Apr 5, 2023

View reviewed changes

fixed readme spaces

513d940

jeremytchang requested a review from jkaster April 5, 2023 01:40

jkaster approved these changes Apr 5, 2023

View reviewed changes

jeremytchang merged commit 308b419 into main Apr 5, 2023

jeremytchang deleted the customheadergo branch April 5, 2023 20:52

looker-open-source-automation-bot mentioned this pull request Apr 5, 2023

chore: release main #1292

Merged

This comment has been minimized.

Sign in to view

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add Go SDK custom header support and README #1288

feat: Add Go SDK custom header support and README #1288

jeremytchang commented Mar 29, 2023 •

edited

Loading

jeremytchang Mar 29, 2023

github-actions bot commented Mar 30, 2023

github-actions bot commented Mar 31, 2023

github-actions bot commented Apr 4, 2023

github-actions bot commented Apr 5, 2023

jkaster left a comment

jkaster Apr 5, 2023

jeremytchang Apr 5, 2023

jkaster Apr 5, 2023

jeremytchang Apr 5, 2023

jkaster Apr 5, 2023

jkaster Apr 5, 2023

jkaster Apr 5, 2023

jeremytchang Apr 5, 2023

github-actions bot commented Apr 5, 2023

github-actions bot commented Apr 5, 2023

github-actions bot commented Apr 5, 2023

jkaster left a comment

This comment has been minimized.


		You can set a custom timeout (in seconds) on Looker Go SDK's requests. The timeout defaults to 120 seconds. A timeout can either be applied to all outgoing requests or per outgoing request.

		#### Timeout for all requests

feat: Add Go SDK custom header support and README #1288

feat: Add Go SDK custom header support and README #1288

Conversation

jeremytchang commented Mar 29, 2023 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Mar 30, 2023

Go Tests

github-actions bot commented Mar 31, 2023

Go Tests

github-actions bot commented Apr 4, 2023

Go Tests

github-actions bot commented Apr 5, 2023

Go Tests

jkaster left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Apr 5, 2023

Go Tests

github-actions bot commented Apr 5, 2023

Go Tests

github-actions bot commented Apr 5, 2023

Go Tests

jkaster left a comment

Choose a reason for hiding this comment

This comment has been minimized.

jeremytchang commented Mar 29, 2023 •

edited

Loading