Propagate Request Headers through API Client by mgyucht · Pull Request #135 · databricks/databricks-sdk-java

mgyucht · 2023-08-14T18:23:49Z

Changes

Based on changes in databricks/databricks-sdk-go#572.

This PR adds initial support for headers in ApiClient.java. The *Impl classes, when making a request, will construct a map of headers to be included in the request. These are passed through a new parameter in each of GET/PUT/POST/PATCH/DELETE.

In the future, to support dynamic headers, we can modify our codegen template to add those fields to the headers map before calling the ApiClient.

Alternatives considered:

Adding headers in request objects directly: this would not require any change in the ApiClient interface. Instead, we would introduce a new annotation for headers, much like what we do for query parameters, and reflectively scan the fields of each request object. We could do this, but for headers that have a fixed value (like Content-Type and Accept), these fields would essentially be private final fields that we could iterate through. Users can never interact with these fields, so it seems a bit unusual to have them in the request structure, a public interface.

Tests

nfx · 2023-08-19T13:55:48Z

    {{if .Response -}}
-      {{- if .Response.ArrayValue -}} return apiClient.getCollection(path, null, {{template "type" .Response.ArrayValue}}.class);
-      {{- else if .Response.MapValue -}} return apiClient.getStringMap(path, request);
+      {{- if .Response.ArrayValue -}} return apiClient.getCollection(path, null, {{template "type" .Response.ArrayValue}}.class, headers);


the diff seems to be larger than it should be. did you try solving it with method overloading?

Suggested change

{{- if .Response.ArrayValue -}} return apiClient.getCollection(path, null, {{template "type" .Response.ArrayValue}}.class, headers);

{{- if .Response.ArrayValue -}} return apiClient.getCollection(path, null, {{template "type" .Response.ArrayValue}}.class{{if .HasRequestHeaders}}, headers{{end}});

mgyucht

Some old comment?

For simplicity, we will always pass headers to the ApiClient.

## Changes In #135 there was a refactor to ApiClient's methods by introducing `prepareRequest`. This accidentally removed the serialization of the body. This PR corrects that issue. ## Tests Ran failing integration tests, and they passed.

## Changes This PR adds support for non-application/json requests and responses. This PR is divided into two parts: **Support setting headers per request.** The main methods of the ApiClient interface are expanded to accept one new parameter,`Map<String, String> headers`, and all callers are expected to pass a map of headers to be included in the request. As the allowed content types for requests and responses are known from the OpenAPI specification, impls must construct the request header map in code generation and pass it to the ApiClient. The default "Content-Type: application/json" header is removed, as each request should specify its own Content-Type and Accept headers. (This is merged in #135.) **Add support for streaming request and response bodies to support non-application/json requests/responses.** Today, serialization of requests is done in the ApiClient. This implies that ApiClient needs to be able to serialize a request purely based on the parameters provided to it via headers, the request body, etc. In this proposal, the serialization is coupled to the request type: InputStream indicates that the client has already serialized the request, so it can be provided directly to the underlying HttpClient library, and any other type will be passed through Jackson's ObjectMapper for serialization to JSON. Internally, request and response bodies are modeled as InputStreams rather than byte arrays to support streaming requests and responses, though for non-streaming requests and responses, a string value for the body is captured in the `debugBody` field of Request and Response, respectively. One important caveat about streaming responses is that callers are required to close the streams when they are done using them. This releases the underlying HTTP connection back to the connection pool for use in subsequent requests. Otherwise, customers could encounter deadlocks waiting for connections. For this, we recommend that users use the try-with-resources model as demonstrated in integration test FilesIT.java. ## Tests - [x] Integration test for Files IT exercises uploading and downloading files using the streaming pathways. - [x] Existing integration tests cover the pre-existing non-streaming pathways.

* Added support for propagating Request Headers through API Client ([#135](#135)). * Added support for InputStream for streaming request and response bodies ([#138](#138)). * Fixed query param serialization for requests with enums ([#140](#140)). Breaking API Changes: * Changed `list()` method for `accountClient.storageCredentials()` service to return `com.databricks.sdk.service.catalog.StorageCredentialInfoList` class. * Removed `workspaceClient.securableTags()` service and all related classes. * Removed `workspaceClient.subentityTags()` service and all related classes. * Renamed `provisioningState` field for `com.databricks.sdk.service.catalog.ConnectionInfo` to `provisioningInfo`. * Removed `instancePoolFleetAttributes` field for `com.databricks.sdk.service.compute.CreateInstancePool`. * Removed `instancePoolFleetAttributes` field for `com.databricks.sdk.service.compute.EditInstancePool`. * Removed `com.databricks.sdk.service.compute.FleetLaunchTemplateOverride` class. * Removed `com.databricks.sdk.service.compute.FleetOnDemandOption` class. * Removed `com.databricks.sdk.service.compute.FleetOnDemandOptionAllocationStrategy` class. * Removed `com.databricks.sdk.service.compute.FleetSpotOption` class. * Removed `com.databricks.sdk.service.compute.FleetSpotOptionAllocationStrategy` class. * Removed `instancePoolFleetAttributes` field for `com.databricks.sdk.service.compute.GetInstancePool`. * Removed `instancePoolFleetAttributes` field for `com.databricks.sdk.service.compute.InstancePoolAndStats`. * Removed `com.databricks.sdk.service.compute.InstancePoolFleetAttributes` class. * Changed `getByName()` method for `workspaceClient.experiments()` service to return `com.databricks.sdk.service.ml.GetExperimentResponse` class. * Changed `getExperiment()` method for `workspaceClient.experiments()` service to return `com.databricks.sdk.service.ml.GetExperimentResponse` class. * Renamed `com.databricks.sdk.service.ml.GetExperimentByNameResponse` class to `com.databricks.sdk.service.ml.GetExperimentResponse`. API Changes: * Added `workspaceClient.modelVersions()` service. * Added `workspaceClient.registeredModels()` service. * Added `browseOnly` field for `com.databricks.sdk.service.catalog.CatalogInfo`. * Added `fullName` field for `com.databricks.sdk.service.catalog.CatalogInfo`. * Added `provisioningInfo` field for `com.databricks.sdk.service.catalog.CatalogInfo`. * Added `securableKind` field for `com.databricks.sdk.service.catalog.CatalogInfo`. * Added `securableType` field for `com.databricks.sdk.service.catalog.CatalogInfo`. * Added `options` field for `com.databricks.sdk.service.catalog.CreateCatalog`. * Added `options` field for `com.databricks.sdk.service.catalog.UpdateCatalog`. * Added `com.databricks.sdk.service.catalog.CatalogInfoSecurableKind` class. * Added `com.databricks.sdk.service.catalog.CreateRegisteredModelRequest` class. * Added `com.databricks.sdk.service.catalog.DeleteAliasRequest` class. * Added `com.databricks.sdk.service.catalog.DeleteModelVersionRequest` class. * Added `com.databricks.sdk.service.catalog.DeleteRegisteredModelRequest` class. * Added `com.databricks.sdk.service.catalog.GetByAliasRequest` class. * Added `com.databricks.sdk.service.catalog.GetModelVersionRequest` class. * Added `com.databricks.sdk.service.catalog.GetRegisteredModelRequest` class. * Added `com.databricks.sdk.service.catalog.ListModelVersionsRequest` class. * Added `com.databricks.sdk.service.catalog.ListModelVersionsResponse` class. * Added `com.databricks.sdk.service.catalog.ListRegisteredModelsRequest` class. * Added `com.databricks.sdk.service.catalog.ListRegisteredModelsResponse` class. * Added `com.databricks.sdk.service.catalog.ModelVersionInfo` class. * Added `com.databricks.sdk.service.catalog.ModelVersionInfoStatus` class. * Added `com.databricks.sdk.service.catalog.ProvisioningInfo` class. * Added `com.databricks.sdk.service.catalog.ProvisioningInfoState` class. * Added `com.databricks.sdk.service.catalog.RegisteredModelAlias` class. * Added `com.databricks.sdk.service.catalog.RegisteredModelInfo` class. * Added `com.databricks.sdk.service.catalog.SetRegisteredModelAliasRequest` class. * Added `com.databricks.sdk.service.catalog.UpdateModelVersionRequest` class. * Added `com.databricks.sdk.service.catalog.UpdateRegisteredModelRequest` class. * Added `volumes` field for `com.databricks.sdk.service.compute.InitScriptInfo`. * Added `com.databricks.sdk.service.compute.VolumesStorageInfo` class. * Added `workspaceClient.files()` service. * Added `com.databricks.sdk.service.files.DeleteFileRequest` class. * Added `com.databricks.sdk.service.files.DownloadRequest` class. * Added `com.databricks.sdk.service.files.DownloadResponse` class. * Added `com.databricks.sdk.service.files.UploadRequest` class. * Added `customTags` field for `com.databricks.sdk.service.provisioning.CreateWorkspaceRequest`. * Added `customTags` field for `com.databricks.sdk.service.provisioning.UpdateWorkspaceRequest`. * Added `customTags` field for `com.databricks.sdk.service.provisioning.Workspace`. * Added `com.databricks.sdk.service.provisioning.CustomTags` class. * Added `parameters` field for `com.databricks.sdk.service.sql.ExecuteStatementRequest`. * Added `rowLimit` field for `com.databricks.sdk.service.sql.ExecuteStatementRequest`. * Added `com.databricks.sdk.service.sql.StatementParameterListItem` class. OpenAPI SHA: 5d0ccbb790d341eae8e85321a685a9e9e2d5bf24, Date: 2023-08-29

mgyucht added 4 commits August 14, 2023 20:07

Propagate headers through API Client

f019a94

remove unused code

269105e

Merge branch 'main' into propagate-headers-to-apiclient

78589ff

Merge branch 'main' into propagate-headers-to-apiclient

6f68bbf

mgyucht changed the title ~~[WIP] Propagate Request Headers through API Client~~ Propagate Request Headers through API Client Aug 18, 2023

regen

9b0c29c

pietern approved these changes Aug 18, 2023

View reviewed changes

mgyucht added 4 commits August 18, 2023 21:38

some fixes

bb84dff

tweaks

de469bb

use main

628a50a

Looking good

596fed6

mgyucht marked this pull request as ready for review August 18, 2023 19:59

mgyucht added 2 commits August 18, 2023 22:00

address comments

0d30290

remove extra file

e390283

nfx previously requested changes Aug 19, 2023

View reviewed changes

mgyucht commented Aug 22, 2023

View reviewed changes

Comment thread .codegen/impl.java.tmpl Outdated

mgyucht added this pull request to the merge queue Aug 22, 2023

Merged via the queue into main with commit 4c6599c Aug 22, 2023

mgyucht deleted the propagate-headers-to-apiclient branch August 22, 2023 13:18

mgyucht mentioned this pull request Aug 22, 2023

Fix refactor to serialize body #141

Merged

mgyucht mentioned this pull request Aug 23, 2023

Support inputstream for request and response bodies #138

Merged

2 tasks

mgyucht mentioned this pull request Aug 29, 2023

Release v0.7.0 #143

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Propagate Request Headers through API Client#135

Propagate Request Headers through API Client#135
mgyucht merged 11 commits into
mainfrom
propagate-headers-to-apiclient

mgyucht commented Aug 14, 2023

Uh oh!

nfx Aug 19, 2023

Uh oh!

mgyucht left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	{{- if .Response.ArrayValue -}} return apiClient.getCollection(path, null, {{template "type" .Response.ArrayValue}}.class, headers);
	{{- if .Response.ArrayValue -}} return apiClient.getCollection(path, null, {{template "type" .Response.ArrayValue}}.class{{if .HasRequestHeaders}}, headers{{end}});

Uh oh!

Conversation

mgyucht commented Aug 14, 2023

Changes

Tests

Uh oh!

nfx Aug 19, 2023

Choose a reason for hiding this comment

Uh oh!

mgyucht left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants