Allow injection of HTTP transport to enable HTTP replayer pattern #697
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 task
nfx
suggested changes
Nov 18, 2023
tcesnik-veza
changed the title
nfx
suggested changes
Nov 20, 2023
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #697 +/- ##
==========================================
+ Coverage 15.87% 15.89% +0.02%
==========================================
Files 94 94
Lines 13773 13775 +2
==========================================
+ Hits 2186 2190 +4
+ Misses 11402 11401 -1
+ Partials 185 184 -1 ☔ View full report in Codecov by Sentry. |
nfx
approved these changes
Nov 20, 2023
mgyucht
approved these changes
Nov 22, 2023
|
@tcesnik-veza thank you for raising this! Can you please setup commit signing using a GPG key and make sure all your commits are signed? See https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits for guidelines. |
## Changes Adds a new function to template generation: `contains` is simply `strings.Contains`. ## Tests <!-- How is this tested? Please see the checklist below and also describe any other relevant tests --> - [ ] `make test` passing - [ ] `make fmt` applied - [ ] relevant integration tests applied Signed-off-by: Tomo Cesnik <tcesnik@veza.com>
## 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 <!-- How is this tested? Please see the checklist below and also describe any other relevant tests --> - [ ] `make test` passing - [ ] `make fmt` applied - [ ] relevant integration tests applied Signed-off-by: Tomo Cesnik <tcesnik@veza.com>
Signed-off-by: Tomo Cesnik <tcesnik@veza.com>
## Changes The test needs to be named TestUcAcc* to run in UC environments. ## Tests <!-- How is this tested? Please see the checklist below and also describe any other relevant tests --> - [ ] `make test` passing - [ ] `make fmt` applied - [ ] relevant integration tests applied Signed-off-by: Tomo Cesnik <tcesnik@veza.com>
Co-authored-by: Serge Smertin <259697+nfx@users.noreply.github.com> Signed-off-by: Tomo Cesnik <tcesnik@veza.com>
Signed-off-by: Tomo Cesnik <tcesnik@veza.com>
Signed-off-by: Tomo Cesnik <tcesnik@veza.com>
Signed-off-by: Tomo Cesnik <tcesnik@veza.com>
Signed-off-by: Tomo Cesnik <tcesnik@veza.com>
Signed-off-by: Tomo Cesnik <tcesnik@veza.com>
auto-merge was automatically disabled
November 22, 2023 14:22
Head branch was pushed to by a user without write access
tcesnik-veza
force-pushed
the
expose-http-transport
branch
from
8056645 to
84b5726
Compare
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 24, 2023
…ent` into `httpclient.ApiClient` (#699) ## Changes This PR decouples resiliency of HTTP client (rate limiting, retries, timeouts, debug logging) from `client.DatabricksClient` and moves it to a dedicated class, allowing to later use the same robust mechanism to refresh AAD tokens. What's changed in `httpclient.ApiClient` compared from `client.DatabricksClient` (originated in Databricks Terraform Provider back in the early 2020): - added native integration with `golang.org/x/oauth2` package for both using token sources in the request as well as injecting a custom `*http.Client` the context for token refreshes. - error mapper is now a pluggable function, default implementation available - error retry checking is now pluggable function, default implementation checks for HTTP 429 and 504 - transient error messages are now configurable per client instance - HTTP request visitors are now configurable per client instance - Explicit and coupled `Do(ctx context.Context, method, path string, headers map[string]string, request, response any, visitors ...func(*http.Request) error) error` became concise and flexible `func (c *ApiClient) Do(ctx context.Context, method, path string, opts ...DoOption) error`. Headers can now be specified with `httpclient.WithHeaders(headers)`, JSON deserialisation can now be specified with `httpclient.WithUnmarshal(&entity)`. - added `httpclient.WithCaptureHeader(key, &value)` to assign headers to a string pointer. This is required to get a proper integration with Azure Container Registry Managed Identity passwordless setup. - body can be specified as `httpclient.WithBody(any)`, that internally sets a special data field. Technically, this could have been a request visitor, but we have special logic for "debug bytes" logging, which is out of scope of this PR change. - removed flawed trailing double-forward-slash trimming (`//`), as it was breaking requests containing full URLs. - Once we merge the #697, we can properly unit-test the implementations of Azure Client Secret and MSI flows This is required for downstream tasks like: - #240 - #566 - #495 - proper solution to #321 instead of hack in d5e57f7 - ## Tests - [x] Only one test was failing in TF provider, fixed in databricks/terraform-provider-databricks#2957
mgyucht
added a commit
that referenced
this pull request
Nov 29, 2023
Major changes: * There has been a major overhaul of error handling. Users can now compare errors in API responses to the well-known error responses defined in the `apierr` package and reexported in the `databricks` package. Users can check whether a specific error was returned, for example `errors.Is(err, databricks.ErrResourceAlreadyExists)`, rather than converting the error to `*APIError` to check the status code and error code. This change is backwards-compatible; users do not need to modify existing error-handling code when upgrading the SDK. See [#682](#682) and [#703](#703) for the changes and https://github.com/databricks/databricks-sdk-go/blob/main/error_alias.go for the full set of errors. Bug fixes: * Handle "no configuration file found at" error during databricks-cli authentication ([#707](#707)). * Introduce `DatabricksEnvironment` and fix Azure MSI auth from ACR, where IMDS doesn't give host environment information ([#700](#700)). * Fix SCIM Pagination default parameters in the Go SDK ([#717](#717)). Other changes: * Update `slog` example with the correct interface ([#694](#694)). * Fixed typo in error message for unknown azure environment ([#701](#701)). * Allow injection of HTTP transport to enable HTTP replayer pattern ([#697](#697)). * Decouple HTTP retries and error mapping mechanics from `DatabricksClient` into `httpclient.ApiClient` ([#699](#699), [#702](#702), [#712](#712)). * Port `qa.HTTPFixtures` to faster transport-level stubs ([#708](#708)). API Changes: * Removed `EnableOptimization` method for [w.Metastores](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#MetastoresAPI) workspace-level service. * Added `PipelineId` field for [catalog.TableInfo](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#TableInfo). * Added `EnablePredictiveOptimization` field for [catalog.UpdateCatalog](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#UpdateCatalog) and [catalog.UpdateSchema](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#UpdateSchema). * Removed [catalog.UpdatePredictiveOptimization](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#UpdatePredictiveOptimization) and [catalog.UpdatePredictiveOptimizationResponse](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#UpdatePredictiveOptimizationResponse). * Added `Description` field for [jobs.CreateJob](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/jobs#CreateJob) and [jobs.JobSettings](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/jobs#JobSettings). * Added `ListNetworkConnectivityConfigurations` and `ListPrivateEndpointRules` method for [a.NetworkConnectivity](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#NetworkConnectivityAPI) account-level service. * Added [settings.ListNccAzurePrivateEndpointRulesResponse](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#ListNccAzurePrivateEndpointRulesResponse), [settings.ListNetworkConnectivityConfigurationsRequest](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#ListNetworkConnectivityConfigurationsRequest), [settings.ListNetworkConnectivityConfigurationsResponse](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#ListNetworkConnectivityConfigurationsResponse), and [settings.ListPrivateEndpointRulesRequest](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#ListPrivateEndpointRulesRequest). * Added `StringSharedAs` field for [sharing.SharedDataObject](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/sharing#SharedDataObject). Internal changes: * Added `contains` method in OpenAPI Generator ([#690](#690)). * Skip recipients tests in Azure ([#692](#692)). * Allow Files API tests to run in UC environments ([#695](#695)). * More cleanup in Unity Catalog integration test ([#719](#719)). OpenAPI SHA: 22f09783eb8a84d52026f856be3b2068f9498db3, Date: 2023-11-23 Dependency updates: * Bump golang.org/x/oauth2 from 0.13.0 to 0.14.0 ([#689](#689)). * Bump google.golang.org/api from 0.150.0 to 0.151.0 ([#698](#698)). * Bump the OpenAPI Spec ([#706](#706)). * Bump golang.org/x/oauth2 from 0.14.0 to 0.15.0 ([#715](#715)). * Bump golang.org/x/time from 0.4.0 to 0.5.0 ([#714](#714)). * Bump google.golang.org/api from 0.151.0 to 0.152.0 ([#716](#716)).
Merged
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 29, 2023
Major changes: * There has been a major overhaul of error handling. Users can now compare errors in API responses to the well-known error responses defined in the `apierr` package and reexported in the `databricks` package. Users can check whether a specific error was returned, for example `errors.Is(err, databricks.ErrResourceAlreadyExists)`, rather than converting the error to `*APIError` to check the status code and error code. This change is backwards-compatible; users do not need to modify existing error-handling code when upgrading the SDK. See [#682](#682) and [#703](#703) for the changes and https://github.com/databricks/databricks-sdk-go/blob/main/error_alias.go for the full set of errors. Bug fixes: * Handle "no configuration file found at" error during databricks-cli authentication ([#707](#707)). * Introduce `DatabricksEnvironment` and fix Azure MSI auth from ACR, where IMDS doesn't give host environment information ([#700](#700)). * Fix SCIM Pagination default parameters in the Go SDK ([#717](#717)). Other changes: * Update `slog` example with the correct interface ([#694](#694)). * Fixed typo in error message for unknown azure environment ([#701](#701)). * Allow injection of HTTP transport to enable HTTP replayer pattern ([#697](#697)). * Decouple HTTP retries and error mapping mechanics from `DatabricksClient` into `httpclient.ApiClient` ([#699](#699), [#702](#702), [#712](#712)). * Port `qa.HTTPFixtures` to faster transport-level stubs ([#708](#708)). API Changes: * Removed `EnableOptimization` method for [w.Metastores](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#MetastoresAPI) workspace-level service. * Added `PipelineId` field for [catalog.TableInfo](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#TableInfo). * Added `EnablePredictiveOptimization` field for [catalog.UpdateCatalog](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#UpdateCatalog) and [catalog.UpdateSchema](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#UpdateSchema). * Removed [catalog.UpdatePredictiveOptimization](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#UpdatePredictiveOptimization) and [catalog.UpdatePredictiveOptimizationResponse](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/catalog#UpdatePredictiveOptimizationResponse). * Added `Description` field for [jobs.CreateJob](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/jobs#CreateJob) and [jobs.JobSettings](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/jobs#JobSettings). * Added `ListNetworkConnectivityConfigurations` and `ListPrivateEndpointRules` method for [a.NetworkConnectivity](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#NetworkConnectivityAPI) account-level service. * Added [settings.ListNccAzurePrivateEndpointRulesResponse](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#ListNccAzurePrivateEndpointRulesResponse), [settings.ListNetworkConnectivityConfigurationsRequest](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#ListNetworkConnectivityConfigurationsRequest), [settings.ListNetworkConnectivityConfigurationsResponse](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#ListNetworkConnectivityConfigurationsResponse), and [settings.ListPrivateEndpointRulesRequest](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/settings#ListPrivateEndpointRulesRequest). * Added `StringSharedAs` field for [sharing.SharedDataObject](https://pkg.go.dev/github.com/databricks/databricks-sdk-go/service/sharing#SharedDataObject). Internal changes: * Added `contains` method in OpenAPI Generator ([#690](#690)). * Skip recipients tests in Azure ([#692](#692)). * Allow Files API tests to run in UC environments ([#695](#695)). * More cleanup in Unity Catalog integration test ([#719](#719)). OpenAPI SHA: 22f09783eb8a84d52026f856be3b2068f9498db3, Date: 2023-11-23 Dependency updates: * Bump golang.org/x/oauth2 from 0.13.0 to 0.14.0 ([#689](#689)). * Bump google.golang.org/api from 0.150.0 to 0.151.0 ([#698](#698)). * Bump the OpenAPI Spec ([#706](#706)). * Bump golang.org/x/oauth2 from 0.14.0 to 0.15.0 ([#715](#715)). * Bump golang.org/x/time from 0.4.0 to 0.5.0 ([#714](#714)). * Bump google.golang.org/api from 0.151.0 to 0.152.0 ([#716](#716)).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Allowing to inject HTTP transport of a client through configuration, to enable use cases involving HTTP replayers: https://github.com/google/go-replayers.