From 5c51a390baef68e468234bd70f12dc53fcd730c3 Mon Sep 17 00:00:00 2001 From: Miles Yucht Date: Mon, 13 Nov 2023 13:49:06 +0100 Subject: [PATCH 01/10] Added `contains` method in OpenAPI Generator (#690) ## Changes Adds a new function to template generation: `contains` is simply `strings.Contains`. ## Tests - [ ] `make test` passing - [ ] `make fmt` applied - [ ] relevant integration tests applied Signed-off-by: Tomo Cesnik --- openapi/code/tmpl_util_funcs.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openapi/code/tmpl_util_funcs.go b/openapi/code/tmpl_util_funcs.go index e771d88fe..551d075ba 100644 --- a/openapi/code/tmpl_util_funcs.go +++ b/openapi/code/tmpl_util_funcs.go @@ -17,7 +17,8 @@ var HelperFuncs = template.FuncMap{ "notLast": func(idx int, a interface{}) bool { return idx+1 != reflect.ValueOf(a).Len() }, - "lower": strings.ToLower, + "contains": strings.Contains, + "lower": strings.ToLower, "lowerFirst": func(s string) string { return strings.ToLower(s[0:1]) + s[1:] }, From a9a2741ec69d42397119e239b73037a27ae35a83 Mon Sep 17 00:00:00 2001 From: Miles Yucht Date: Wed, 15 Nov 2023 13:11:47 +0100 Subject: [PATCH 02/10] Skip recipients tests in Azure (#692) ## Changes The recipient activation flow in the SDK doesn't work properly today because the API requires that this request is sent to the host specified in the activation URL of the token generated when creating the recipient. Currently, the SDK only supports using the host from the static configuration. This does work in AWS and GCP today but purely by coincidence, so for now we will stop running these tests in Azure. ## Tests - [ ] `make test` passing - [ ] `make fmt` applied - [ ] relevant integration tests applied Signed-off-by: Tomo Cesnik --- internal/sharing_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/sharing_test.go b/internal/sharing_test.go index 8db0ae977..b68bf10f4 100644 --- a/internal/sharing_test.go +++ b/internal/sharing_test.go @@ -55,6 +55,9 @@ func TestUcAccProviders(t *testing.T) { // TODO: remove NoTranspile func TestUcAccRecipientActivationNoTranspile(t *testing.T) { ctx, w := ucwsTest(t) + if w.Config.IsAzure() { + skipf(t)("temporarily skipping this test on Azure until RetrieveToken uses the same host as specified in the activation URL") + } created, err := w.Recipients.Create(ctx, sharing.CreateRecipient{ Name: RandomName("go-sdk-"), From fc51d766a5612f99fbe47dc830c34db09275f390 Mon Sep 17 00:00:00 2001 From: Tomo Cesnik Date: Fri, 17 Nov 2023 15:29:17 +0100 Subject: [PATCH 03/10] Allowed injection of HTTP transport to enable HTTP replayer pattern Signed-off-by: Tomo Cesnik --- client/client.go | 38 +++++++++++++++++++++----------------- config/config.go | 2 ++ 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/client/client.go b/client/client.go index 52744ba02..10417b6de 100644 --- a/client/client.go +++ b/client/client.go @@ -35,6 +35,25 @@ func New(cfg *config.Config) (*DatabricksClient, error) { httpTimeout := time.Duration(orDefault(cfg.HTTPTimeoutSeconds, 60)) * time.Second rateLimiter := rate.NewLimiter(rate.Limit(orDefault(cfg.RateLimitPerSecond, 15)), 1) debugTruncateBytes := orDefault(cfg.DebugTruncateBytes, 96) + httpTransport := cfg.HTTPTransport + if httpTransport == nil { + httpTransport = &http.Transport{ + Proxy: http.ProxyFromEnvironment, + DialContext: (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + }).DialContext, + ForceAttemptHTTP2: true, + MaxIdleConns: 100, + MaxIdleConnsPerHost: runtime.GOMAXPROCS(0) + 1, + IdleConnTimeout: 180 * time.Second, + TLSHandshakeTimeout: 30 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: cfg.InsecureSkipVerify, + }, + } + } return &DatabricksClient{ Config: cfg, debugHeaders: cfg.DebugHeaders, @@ -42,23 +61,8 @@ func New(cfg *config.Config) (*DatabricksClient, error) { retryTimeout: retryTimeout, rateLimiter: rateLimiter, httpClient: &http.Client{ - Timeout: httpTimeout, - Transport: &http.Transport{ - Proxy: http.ProxyFromEnvironment, - DialContext: (&net.Dialer{ - Timeout: 30 * time.Second, - KeepAlive: 30 * time.Second, - }).DialContext, - ForceAttemptHTTP2: true, - MaxIdleConns: 100, - MaxIdleConnsPerHost: runtime.GOMAXPROCS(0) + 1, - IdleConnTimeout: 180 * time.Second, - TLSHandshakeTimeout: 30 * time.Second, - ExpectContinueTimeout: 1 * time.Second, - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: cfg.InsecureSkipVerify, - }, - }, + Timeout: httpTimeout, + Transport: httpTransport, }, }, nil } diff --git a/config/config.go b/config/config.go index 26fd3bc83..505321eb0 100644 --- a/config/config.go +++ b/config/config.go @@ -105,6 +105,8 @@ type Config struct { // Number of seconds to keep retrying HTTP requests. Default is 300 (5 minutes) RetryTimeoutSeconds int `name:"retry_timeout_seconds" auth:"-"` + HTTPTransport *http.Transport + Loaders []Loader // marker for configuration resolving From 0083eccfff7b1fb56b61f75690ace83ed261c566 Mon Sep 17 00:00:00 2001 From: Miles Yucht Date: Fri, 17 Nov 2023 15:59:13 +0100 Subject: [PATCH 04/10] Allow Files API tests to run in UC environments (#695) ## Changes The test needs to be named TestUcAcc* to run in UC environments. ## Tests - [ ] `make test` passing - [ ] `make fmt` applied - [ ] relevant integration tests applied Signed-off-by: Tomo Cesnik --- internal/files_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/files_test.go b/internal/files_test.go index 0ae1202d3..d17f2cf78 100644 --- a/internal/files_test.go +++ b/internal/files_test.go @@ -26,7 +26,7 @@ func (buf hashable) Hash() uint32 { return h.Sum32() } -func TestAccFilesAPI(t *testing.T) { +func TestUcAccFilesAPI(t *testing.T) { ctx, w := ucwsTest(t) schema, err := w.Schemas.Create(ctx, catalog.CreateSchema{ From 197b76ce25bf14cca1f338ea80806e544e359475 Mon Sep 17 00:00:00 2001 From: Tomo Cesnik <121861383+tcesnik-veza@users.noreply.github.com> Date: Mon, 20 Nov 2023 09:57:15 +0100 Subject: [PATCH 05/10] Using RoundTripper instead of Transport Co-authored-by: Serge Smertin <259697+nfx@users.noreply.github.com> Signed-off-by: Tomo Cesnik --- config/config.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/config.go b/config/config.go index 505321eb0..e658aafe1 100644 --- a/config/config.go +++ b/config/config.go @@ -105,7 +105,8 @@ type Config struct { // Number of seconds to keep retrying HTTP requests. Default is 300 (5 minutes) RetryTimeoutSeconds int `name:"retry_timeout_seconds" auth:"-"` - HTTPTransport *http.Transport + // HTTPTransport can be overriden for unit testing and together with tooling like https://github.com/google/go-replayers + HTTPTransport http.RoundTripper Loaders []Loader From cc056f91e901dda8e37a6232e4a07bae9660629c Mon Sep 17 00:00:00 2001 From: Tomo Cesnik Date: Mon, 20 Nov 2023 11:55:00 +0100 Subject: [PATCH 06/10] Added unit test for injected transport Signed-off-by: Tomo Cesnik --- client/client_test.go | 33 +++++++++++++++++++++++++++++++++ go.mod | 1 + go.sum | 1 + 3 files changed, 35 insertions(+) diff --git a/client/client_test.go b/client/client_test.go index d2b9c645d..9d08c4d53 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -16,6 +16,8 @@ import ( "github.com/databricks/databricks-sdk-go/config" "github.com/databricks/databricks-sdk-go/logger" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" "golang.org/x/time/rate" ) @@ -727,3 +729,34 @@ func TestRetryGetRequest(t *testing.T) { assert.Equal(t, "succeeded", respBytes.String()) assert.True(t, succeed) } + +type mockedTransport struct { + mock.Mock +} + +func (m *mockedTransport) RoundTrip(request *http.Request) (*http.Response, error) { + m.Called(request) + argResponse := &http.Response{Request: request} + return argResponse, nil +} + +func TestHttpTransport(t *testing.T) { + const methodName = "RoundTrip" + + mockedTransport := new(mockedTransport) + matchedBy := mock.MatchedBy(func(arg any) bool { + _, ok := arg.(*http.Request) + return ok + }) + mockedTransport.On(methodName, matchedBy).Return() + + cfg := config.NewMockConfig(func(r *http.Request) error { return nil }) + cfg.HTTPTransport = mockedTransport + client, err := New(cfg) + require.NoError(t, err) + + err = client.Do(context.Background(), "GET", "/a", nil, nil, bytes.Buffer{}) + require.NoError(t, err) + + mockedTransport.AssertExpectations(t) +} diff --git a/go.mod b/go.mod index 892ab66c1..589d55b42 100644 --- a/go.mod +++ b/go.mod @@ -22,6 +22,7 @@ require ( github.com/google/s2a-go v0.1.7 // indirect github.com/googleapis/enterprise-certificate-proxy v0.3.2 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/stretchr/objx v0.5.0 // indirect go.opencensus.io v0.24.0 // indirect golang.org/x/crypto v0.15.0 // indirect golang.org/x/net v0.18.0 // indirect diff --git a/go.sum b/go.sum index d17cccdf9..4fcb45a51 100644 --- a/go.sum +++ b/go.sum @@ -55,6 +55,7 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= From 77746572fffac91aaef670b78125ccc330765ea4 Mon Sep 17 00:00:00 2001 From: Tomo Cesnik Date: Mon, 20 Nov 2023 14:35:07 +0100 Subject: [PATCH 07/10] Not using mocking library anymore Signed-off-by: Tomo Cesnik --- client/client_test.go | 26 ++++++++------------------ go.mod | 1 - 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 9d08c4d53..ccf809767 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -16,7 +16,6 @@ import ( "github.com/databricks/databricks-sdk-go/config" "github.com/databricks/databricks-sdk-go/logger" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "golang.org/x/time/rate" ) @@ -730,33 +729,24 @@ func TestRetryGetRequest(t *testing.T) { assert.True(t, succeed) } -type mockedTransport struct { - mock.Mock -} - -func (m *mockedTransport) RoundTrip(request *http.Request) (*http.Response, error) { - m.Called(request) - argResponse := &http.Response{Request: request} - return argResponse, nil +func (cb hc) RoundTrip(r *http.Request) (*http.Response, error) { + return cb(r) } func TestHttpTransport(t *testing.T) { const methodName = "RoundTrip" - mockedTransport := new(mockedTransport) - matchedBy := mock.MatchedBy(func(arg any) bool { - _, ok := arg.(*http.Request) - return ok - }) - mockedTransport.On(methodName, matchedBy).Return() - + calledMock := false cfg := config.NewMockConfig(func(r *http.Request) error { return nil }) - cfg.HTTPTransport = mockedTransport + cfg.HTTPTransport = hc(func(r *http.Request) (*http.Response, error) { + calledMock = true + return &http.Response{Request: r}, nil + }) client, err := New(cfg) require.NoError(t, err) err = client.Do(context.Background(), "GET", "/a", nil, nil, bytes.Buffer{}) require.NoError(t, err) - mockedTransport.AssertExpectations(t) + assert.True(t, calledMock) } diff --git a/go.mod b/go.mod index 589d55b42..892ab66c1 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,6 @@ require ( github.com/google/s2a-go v0.1.7 // indirect github.com/googleapis/enterprise-certificate-proxy v0.3.2 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/stretchr/objx v0.5.0 // indirect go.opencensus.io v0.24.0 // indirect golang.org/x/crypto v0.15.0 // indirect golang.org/x/net v0.18.0 // indirect From 4bf243b8ac363a6521e2b97aad941544d125f417 Mon Sep 17 00:00:00 2001 From: Tomo Cesnik Date: Mon, 20 Nov 2023 14:36:09 +0100 Subject: [PATCH 08/10] Removing go.sum change Signed-off-by: Tomo Cesnik --- go.sum | 1 - 1 file changed, 1 deletion(-) diff --git a/go.sum b/go.sum index 4fcb45a51..d17cccdf9 100644 --- a/go.sum +++ b/go.sum @@ -55,7 +55,6 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= -github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= From fda5fa0a375317a156d2239261d9578ffdbad0e8 Mon Sep 17 00:00:00 2001 From: Tomo Cesnik Date: Mon, 20 Nov 2023 14:37:11 +0100 Subject: [PATCH 09/10] Cleanup of unused const Signed-off-by: Tomo Cesnik --- client/client_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index ccf809767..348e32317 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -734,8 +734,6 @@ func (cb hc) RoundTrip(r *http.Request) (*http.Response, error) { } func TestHttpTransport(t *testing.T) { - const methodName = "RoundTrip" - calledMock := false cfg := config.NewMockConfig(func(r *http.Request) error { return nil }) cfg.HTTPTransport = hc(func(r *http.Request) (*http.Response, error) { From 84b5726d16b4eb7f730a5bfc27b1fe3c386841e7 Mon Sep 17 00:00:00 2001 From: Tomo Cesnik Date: Mon, 20 Nov 2023 15:45:00 +0100 Subject: [PATCH 10/10] Corrected formatting Signed-off-by: Tomo Cesnik --- config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/config.go b/config/config.go index e658aafe1..abf42b5eb 100644 --- a/config/config.go +++ b/config/config.go @@ -105,7 +105,7 @@ type Config struct { // Number of seconds to keep retrying HTTP requests. Default is 300 (5 minutes) RetryTimeoutSeconds int `name:"retry_timeout_seconds" auth:"-"` - // HTTPTransport can be overriden for unit testing and together with tooling like https://github.com/google/go-replayers + // HTTPTransport can be overriden for unit testing and together with tooling like https://github.com/google/go-replayers HTTPTransport http.RoundTripper Loaders []Loader