[Fix] Support Query parameters for all HTTP operations #1124
Conversation
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
client/client_test.go
Outdated
| }, | ||
| nil, | ||
| map[string]string{ | ||
| "c": "d", | ||
| }, nil) |
There was a problem hiding this comment.
This formatting is hard to read and not consistent, let's do something slightly more idiomatic:
err = c.Do(
context.Background(),
"GET",
"/a/b",
map[string]string{"e": "f"},
nil,
map[string]string{"c": "d"},
nil,
)Also applies to other tests.
client/client_test.go
Outdated
| }, | ||
| nil, | ||
| map[string]string{ | ||
| "c": "d", |
There was a problem hiding this comment.
Can we also cover cases where the value is not a string?
hectorcast-db
force-pushed
the
hectorcast-db/fix-query
branch
from
8972af3 to
279f0c5
Compare
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
hectorcast-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
| @@ -141,3 +141,26 @@ func TestEncodeMultiSegmentPathParameter(t *testing.T) { | |||
| // # and ? should be encoded. | |||
| assert.Equal(t, "a%23b%3Fc", EncodeMultiSegmentPathParameter("a#b?c")) | |||
| } | |||
|
|
|||
| func TestMakeRequestBodyExplicitQueryParams(t *testing.T) { | |||
| type x struct { | |||
There was a problem hiding this comment.
To be clear, the first scenario should not happen, IIUC. For GET/HEAD/DELETE requests, extra query parameters are never passed in.
Could the second scenario happen, where the request contains url tags that are overridden by the explicit query parameters?
| @@ -63,12 +63,13 @@ func (c *DatabricksClient) GetOAuthToken(ctx context.Context, authDetails string | |||
|
|
|||
| // Do sends an HTTP request against path. | |||
| func (c *DatabricksClient) Do(ctx context.Context, method, path string, | |||
| headers map[string]string, request, response any, | |||
| headers map[string]string, queryParams map[string]any, request, response any, | |||
There was a problem hiding this comment.
For external clients (like Terraform), the migration path is to pass nil for GET/HEAD/DELETE requests and to explicitly pass the query parameters here?
What changes are proposed in this pull request?
Support Query parameters for all HTTP operations. Previously, only GET/HEAD/DELETE were supported.
How is this tested?