Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support BinaryIO for streaming request and response bodies #303

Merged
merged 32 commits into from
Aug 23, 2023

Conversation

mgyucht
Copy link
Contributor

@mgyucht mgyucht commented Aug 19, 2023

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 do method of the ApiClient class is expanded to accept one new parameter, headers: dict, 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, callers 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 done in #302.

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 specified by the caller depending on whether the body or data field of do() is used: data should be used for already-serialized data, such as BinaryIO or file-like objects, and body should be used for JSON objects.

One important caveat about streaming responses is that callers are required to close the streams when they are done using them. To do this, we may need to expose the raw Response object so that users can use with w.service.method() as response: to automatically close the response when they are done reading.

Tests

  • make test run locally
  • make fmt applied
  • relevant integration tests applied

@codecov-commenter
Copy link

codecov-commenter commented Aug 19, 2023

Codecov Report

Patch coverage: 0.18% and no project coverage change.

Comparison is base (27356dd) 51.68% compared to head (3778a7b) 51.68%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #303   +/-   ##
=======================================
  Coverage   51.68%   51.68%           
=======================================
  Files          33       33           
  Lines       20800    20801    +1     
=======================================
+ Hits        10751    10752    +1     
  Misses      10049    10049           
Files Changed Coverage Δ
databricks/sdk/service/billing.py 51.88% <0.00%> (ø)
databricks/sdk/service/catalog.py 50.74% <0.00%> (ø)
databricks/sdk/service/compute.py 51.11% <0.00%> (ø)
databricks/sdk/service/files.py 47.00% <0.00%> (ø)
databricks/sdk/service/iam.py 41.55% <0.00%> (ø)
databricks/sdk/service/jobs.py 54.32% <0.00%> (ø)
databricks/sdk/service/ml.py 47.55% <0.00%> (ø)
databricks/sdk/service/oauth2.py 49.51% <0.00%> (ø)
databricks/sdk/service/pipelines.py 49.35% <0.00%> (ø)
databricks/sdk/service/provisioning.py 46.92% <0.00%> (ø)
... and 6 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mgyucht mgyucht changed the title [WIP] Support bytesio for request and response bodies Support BinaryIO for streaming request and response bodies Aug 19, 2023
@mgyucht mgyucht changed the base branch from main to propagate-headers-to-apiclient August 19, 2023 06:10
…r-request-and-response-bodies

# Conflicts:
#	.codegen/service.py.tmpl
#	databricks/sdk/service/billing.py
#	databricks/sdk/service/catalog.py
#	databricks/sdk/service/compute.py
#	databricks/sdk/service/files.py
#	databricks/sdk/service/iam.py
#	databricks/sdk/service/jobs.py
#	databricks/sdk/service/ml.py
#	databricks/sdk/service/oauth2.py
#	databricks/sdk/service/pipelines.py
#	databricks/sdk/service/provisioning.py
#	databricks/sdk/service/serving.py
#	databricks/sdk/service/settings.py
#	databricks/sdk/service/sharing.py
#	databricks/sdk/service/sql.py
#	databricks/sdk/service/workspace.py
Base automatically changed from propagate-headers-to-apiclient to main August 23, 2023 08:54
import time
import io
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this?

Comment on lines 201 to 207
{{if .Request.HasJsonField -}}
body = {}{{ end }}{{if .Request.HasQueryField}}
query = {}{{ end }}{{ range .Request.Fields }}{{ if not .IsPath -}}
{{ if .IsJson }}
if {{.SnakeName}} is not None: body['{{.Name}}'] = {{template "method-param-bind" .}}{{end}}{{ if .IsQuery }}
if {{.SnakeName}} is not None: query['{{.Name}}'] = {{template "method-param-bind" .}}{{end}}
{{- end}}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I safely revert this?

@mgyucht mgyucht requested a review from tanmay-db August 23, 2023 09:50
Copy link
Contributor

@tanmay-db tanmay-db left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went over the PR with Miles, LGTM

@mgyucht mgyucht enabled auto-merge August 23, 2023 10:25
@mgyucht mgyucht added this pull request to the merge queue Aug 23, 2023
Merged via the queue into main with commit 821b02a Aug 23, 2023
@mgyucht mgyucht deleted the support-bytesio-for-request-and-response-bodies branch August 23, 2023 10:28
mgyucht added a commit that referenced this pull request Aug 29, 2023
* Added support for GZIP'ed streaming responses ([#306](#306)).
* Added support for per-method request headers to ApiClient ([#302](#302)).
* Added support for BinaryIO for streaming request and response bodies ([#303](#303)).
* Added a link to the API reference ([#311](#311)).
* Check workspaceUrl explicitly in runtime repl auth ([#312](#312)).

Breaking Changes:
 * Added support for the Files API (using application/octet-stream) in OpenAPI. The names of parameters have changed from `src` to `contents`, and `w.files.download()` now returns a `files.DownloadResponse`, whose `contents` field is a `BinaryIO` object. When reading a download, the user must explicitly close this object to allow the connection to return to the connection pool.

Breaking API Changes:
 * Changed `list()` method for [a.account_storage_credentials](https://databricks-sdk-py.readthedocs.io/en/latest/account/account_storage_credentials.html) account-level service to return `databricks.sdk.service.catalog.StorageCredentialInfoList` dataclass.
 * Removed [w.securable_tags](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/securable_tags.html) workspace-level service and all associated classes.
 * Removed [w.subentity_tags](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/subentity_tags.html) workspace-level service and all associated classes.
 * Removed `instance_pool_fleet_attributes` field for `databricks.sdk.service.compute.CreateInstancePool`.
 * Removed `instance_pool_fleet_attributes` field for `databricks.sdk.service.compute.EditInstancePool`.
 * Removed `databricks.sdk.service.compute.FleetLaunchTemplateOverride` dataclass.
 * Removed `databricks.sdk.service.compute.FleetOnDemandOption` dataclass.
 * Removed `databricks.sdk.service.compute.FleetOnDemandOptionAllocationStrategy` dataclass.
 * Removed `databricks.sdk.service.compute.FleetSpotOption` dataclass.
 * Removed `databricks.sdk.service.compute.FleetSpotOptionAllocationStrategy` dataclass.
 * Removed `instance_pool_fleet_attributes` field for `databricks.sdk.service.compute.GetInstancePool`.
 * Removed `instance_pool_fleet_attributes` field for `databricks.sdk.service.compute.InstancePoolAndStats`.
 * Removed `databricks.sdk.service.compute.InstancePoolFleetAttributes` dataclass.
 * Changed `get_by_name()` method for [w.experiments](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/experiments.html) workspace-level service to return `databricks.sdk.service.ml.GetExperimentResponse` dataclass.
 * Changed `get_experiment()` method for [w.experiments](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/experiments.html) workspace-level service to return `databricks.sdk.service.ml.GetExperimentResponse` dataclass.
 * Renamed `databricks.sdk.service.ml.GetExperimentByNameResponse` dataclass to `databricks.sdk.service.ml.GetExperimentResponse`.
 * Renamed `databricks.sdk.service.catalog.ProvisioningState` to `databricks.sdk.service.catalog.ProvisioningInfoState` dataclass.

API Changes:
 * Added [w.model_versions](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/model_versions.html) workspace-level service.
 * Added [w.registered_models](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/registered_models.html) workspace-level service.
 * Added `browse_only` field for `databricks.sdk.service.catalog.CatalogInfo`.
 * Added `full_name` field for `databricks.sdk.service.catalog.CatalogInfo`.
 * Added `provisioning_info` field for `databricks.sdk.service.catalog.CatalogInfo`.
 * Added `securable_kind` field for `databricks.sdk.service.catalog.CatalogInfo`.
 * Added `securable_type` field for `databricks.sdk.service.catalog.CatalogInfo`.
 * Added `provisioning_info` field for `databricks.sdk.service.catalog.ConnectionInfo`.
 * Added `options` field for `databricks.sdk.service.catalog.CreateCatalog`.
 * Added `options` field for `databricks.sdk.service.catalog.UpdateCatalog`.
 * Added `databricks.sdk.service.catalog.CatalogInfoSecurableKind` dataclass.
 * Added `databricks.sdk.service.catalog.CreateRegisteredModelRequest` dataclass.
 * Added `databricks.sdk.service.catalog.DeleteAliasRequest` dataclass.
 * Added `databricks.sdk.service.catalog.DeleteModelVersionRequest` dataclass.
 * Added `databricks.sdk.service.catalog.DeleteRegisteredModelRequest` dataclass.
 * Added `databricks.sdk.service.catalog.GetByAliasRequest` dataclass.
 * Added `databricks.sdk.service.catalog.GetModelVersionRequest` dataclass.
 * Added `databricks.sdk.service.catalog.GetRegisteredModelRequest` dataclass.
 * Added `databricks.sdk.service.catalog.ListModelVersionsRequest` dataclass.
 * Added `databricks.sdk.service.catalog.ListModelVersionsResponse` dataclass.
 * Added `databricks.sdk.service.catalog.ListRegisteredModelsRequest` dataclass.
 * Added `databricks.sdk.service.catalog.ListRegisteredModelsResponse` dataclass.
 * Added `databricks.sdk.service.catalog.ModelVersionInfo` dataclass.
 * Added `databricks.sdk.service.catalog.ModelVersionInfoStatus` dataclass.
 * Added `databricks.sdk.service.catalog.ProvisioningInfo` dataclass.
 * Added `databricks.sdk.service.catalog.RegisteredModelAlias` dataclass.
 * Added `databricks.sdk.service.catalog.RegisteredModelInfo` dataclass.
 * Added `databricks.sdk.service.catalog.SetRegisteredModelAliasRequest` dataclass.
 * Added `databricks.sdk.service.catalog.UpdateModelVersionRequest` dataclass.
 * Added `databricks.sdk.service.catalog.UpdateRegisteredModelRequest` dataclass.
 * Added `volumes` field for `databricks.sdk.service.compute.InitScriptInfo`.
 * Added `databricks.sdk.service.compute.VolumesStorageInfo` dataclass.
 * Added [w.files](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/files.html) workspace-level service.
 * Added `databricks.sdk.service.files.DeleteFileRequest` dataclass.
 * Added `databricks.sdk.service.files.DownloadRequest` dataclass.
 * Added `databricks.sdk.service.files.DownloadResponse` dataclass.
 * Added `databricks.sdk.service.files.UploadRequest` dataclass.
 * Added `custom_tags` field for `databricks.sdk.service.provisioning.CreateWorkspaceRequest`.
 * Added `custom_tags` field for `databricks.sdk.service.provisioning.UpdateWorkspaceRequest`.
 * Added `custom_tags` field for `databricks.sdk.service.provisioning.Workspace`.
 * Added `databricks.sdk.service.provisioning.CustomTags` dataclass.
 * Added `parameters` field for `databricks.sdk.service.sql.ExecuteStatementRequest`.
 * Added `row_limit` field for `databricks.sdk.service.sql.ExecuteStatementRequest`.
 * Added `databricks.sdk.service.sql.StatementParameterListItem` dataclass.

SDK Internal Changes:
 * Skip Graviton runtimes for testing notebook native auth ([#294](#294)).
 * Fixed integration tests to not use beta DBR ([#309](#309)).

OpenAPI SHA: 5d0ccbb790d341eae8e85321a685a9e9e2d5bf24, Date: 2023-08-29
@mgyucht mgyucht mentioned this pull request Aug 29, 2023
github-merge-queue bot pushed a commit that referenced this pull request Aug 29, 2023
* Added support for GZIP'ed streaming responses
([#306](#306)).
* Added support for per-method request headers to ApiClient
([#302](#302)).
* Added support for BinaryIO for streaming request and response bodies
([#303](#303)).
* Added a link to the API reference
([#311](#311)).
* Check workspaceUrl explicitly in runtime repl auth
([#312](#312)).

Breaking Changes:
* Added support for the Files API (using application/octet-stream) in
OpenAPI. The names of parameters have changed from `src` to `contents`,
and `w.files.download()` now returns a `files.DownloadResponse`, whose
`contents` field is a `BinaryIO` object. When reading a download, the
user must explicitly close this object to allow the connection to return
to the connection pool.

Breaking API Changes:
* Changed `list()` method for
[a.account_storage_credentials](https://databricks-sdk-py.readthedocs.io/en/latest/account/account_storage_credentials.html)
account-level service to return
`databricks.sdk.service.catalog.StorageCredentialInfoList` dataclass.
* Removed
[w.securable_tags](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/securable_tags.html)
workspace-level service and all associated classes.
* Removed
[w.subentity_tags](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/subentity_tags.html)
workspace-level service and all associated classes.
* Removed `instance_pool_fleet_attributes` field for
`databricks.sdk.service.compute.CreateInstancePool`.
* Removed `instance_pool_fleet_attributes` field for
`databricks.sdk.service.compute.EditInstancePool`.
* Removed `databricks.sdk.service.compute.FleetLaunchTemplateOverride`
dataclass.
* Removed `databricks.sdk.service.compute.FleetOnDemandOption`
dataclass.
* Removed
`databricks.sdk.service.compute.FleetOnDemandOptionAllocationStrategy`
dataclass.
 * Removed `databricks.sdk.service.compute.FleetSpotOption` dataclass.
* Removed
`databricks.sdk.service.compute.FleetSpotOptionAllocationStrategy`
dataclass.
* Removed `instance_pool_fleet_attributes` field for
`databricks.sdk.service.compute.GetInstancePool`.
* Removed `instance_pool_fleet_attributes` field for
`databricks.sdk.service.compute.InstancePoolAndStats`.
* Removed `databricks.sdk.service.compute.InstancePoolFleetAttributes`
dataclass.
* Changed `get_by_name()` method for
[w.experiments](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/experiments.html)
workspace-level service to return
`databricks.sdk.service.ml.GetExperimentResponse` dataclass.
* Changed `get_experiment()` method for
[w.experiments](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/experiments.html)
workspace-level service to return
`databricks.sdk.service.ml.GetExperimentResponse` dataclass.
* Renamed `databricks.sdk.service.ml.GetExperimentByNameResponse`
dataclass to `databricks.sdk.service.ml.GetExperimentResponse`.
* Renamed `databricks.sdk.service.catalog.ProvisioningState` to
`databricks.sdk.service.catalog.ProvisioningInfoState` dataclass.

API Changes:
* Added
[w.model_versions](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/model_versions.html)
workspace-level service.
* Added
[w.registered_models](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/registered_models.html)
workspace-level service.
* Added `browse_only` field for
`databricks.sdk.service.catalog.CatalogInfo`.
* Added `full_name` field for
`databricks.sdk.service.catalog.CatalogInfo`.
* Added `provisioning_info` field for
`databricks.sdk.service.catalog.CatalogInfo`.
* Added `securable_kind` field for
`databricks.sdk.service.catalog.CatalogInfo`.
* Added `securable_type` field for
`databricks.sdk.service.catalog.CatalogInfo`.
* Added `provisioning_info` field for
`databricks.sdk.service.catalog.ConnectionInfo`.
* Added `options` field for
`databricks.sdk.service.catalog.CreateCatalog`.
* Added `options` field for
`databricks.sdk.service.catalog.UpdateCatalog`.
* Added `databricks.sdk.service.catalog.CatalogInfoSecurableKind`
dataclass.
* Added `databricks.sdk.service.catalog.CreateRegisteredModelRequest`
dataclass.
 * Added `databricks.sdk.service.catalog.DeleteAliasRequest` dataclass.
* Added `databricks.sdk.service.catalog.DeleteModelVersionRequest`
dataclass.
* Added `databricks.sdk.service.catalog.DeleteRegisteredModelRequest`
dataclass.
 * Added `databricks.sdk.service.catalog.GetByAliasRequest` dataclass.
* Added `databricks.sdk.service.catalog.GetModelVersionRequest`
dataclass.
* Added `databricks.sdk.service.catalog.GetRegisteredModelRequest`
dataclass.
* Added `databricks.sdk.service.catalog.ListModelVersionsRequest`
dataclass.
* Added `databricks.sdk.service.catalog.ListModelVersionsResponse`
dataclass.
* Added `databricks.sdk.service.catalog.ListRegisteredModelsRequest`
dataclass.
* Added `databricks.sdk.service.catalog.ListRegisteredModelsResponse`
dataclass.
 * Added `databricks.sdk.service.catalog.ModelVersionInfo` dataclass.
* Added `databricks.sdk.service.catalog.ModelVersionInfoStatus`
dataclass.
 * Added `databricks.sdk.service.catalog.ProvisioningInfo` dataclass.
* Added `databricks.sdk.service.catalog.RegisteredModelAlias` dataclass.
 * Added `databricks.sdk.service.catalog.RegisteredModelInfo` dataclass.
* Added `databricks.sdk.service.catalog.SetRegisteredModelAliasRequest`
dataclass.
* Added `databricks.sdk.service.catalog.UpdateModelVersionRequest`
dataclass.
* Added `databricks.sdk.service.catalog.UpdateRegisteredModelRequest`
dataclass.
* Added `volumes` field for
`databricks.sdk.service.compute.InitScriptInfo`.
 * Added `databricks.sdk.service.compute.VolumesStorageInfo` dataclass.
* Added
[w.files](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/files.html)
workspace-level service.
 * Added `databricks.sdk.service.files.DeleteFileRequest` dataclass.
 * Added `databricks.sdk.service.files.DownloadRequest` dataclass.
 * Added `databricks.sdk.service.files.DownloadResponse` dataclass.
 * Added `databricks.sdk.service.files.UploadRequest` dataclass.
* Added `custom_tags` field for
`databricks.sdk.service.provisioning.CreateWorkspaceRequest`.
* Added `custom_tags` field for
`databricks.sdk.service.provisioning.UpdateWorkspaceRequest`.
* Added `custom_tags` field for
`databricks.sdk.service.provisioning.Workspace`.
 * Added `databricks.sdk.service.provisioning.CustomTags` dataclass.
* Added `parameters` field for
`databricks.sdk.service.sql.ExecuteStatementRequest`.
* Added `row_limit` field for
`databricks.sdk.service.sql.ExecuteStatementRequest`.
* Added `databricks.sdk.service.sql.StatementParameterListItem`
dataclass.

SDK Internal Changes:
* Skip Graviton runtimes for testing notebook native auth
([#294](#294)).
* Fixed integration tests to not use beta DBR
([#309](#309)).

OpenAPI SHA: 5d0ccbb790d341eae8e85321a685a9e9e2d5bf24, Date: 2023-08-29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants