-
Notifications
You must be signed in to change notification settings - Fork 130
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
Fix null body response to empty in ApiClient #579
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test?
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #579 +/- ##
==========================================
+ Coverage 57.61% 57.62% +0.01%
==========================================
Files 47 47
Lines 32648 32650 +2
==========================================
+ Hits 18810 18815 +5
+ Misses 13838 13835 -3 ☔ View full report in Codecov by Sentry. |
@mgyucht, I have asked on slack for confirmation on the fix, since the API is private, we might need to enable it for our test infra and then repro this, it would be some work, once confirmed I will add unit test. Me and @edwardfeng-db are working on other issues in the meantime parallelly. |
This PR breaks backwards compatibility for databrickslabs/blueprint downstream. See build logs for more details. Running from downstreams #108 |
Note: The failing test: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks all for quick response and feedback on this one. This bug is one of our final blockers to shipping a new integration. 🥳
* Fix null body response to empty in ApiClient ([#579](#579)). API Changes: * Changed `list()` method for [w.connections](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/connections.html) workspace-level service to require request of `databricks.sdk.service.catalog.ListConnectionsRequest` dataclass. * Removed [w.lakehouse_monitors](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/lakehouse_monitors.html) workspace-level service. * Added [w.quality_monitors](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/quality_monitors.html) workspace-level service. * Removed `databricks.sdk.service.catalog.DeleteLakehouseMonitorRequest` dataclass. * Removed `databricks.sdk.service.catalog.GetLakehouseMonitorRequest` dataclass. * Added `next_page_token` field for `databricks.sdk.service.catalog.ListConnectionsResponse`. * Added `dashboard_id` field for `databricks.sdk.service.catalog.UpdateMonitor`. * Added `databricks.sdk.service.catalog.DeleteQualityMonitorRequest` dataclass. * Added `databricks.sdk.service.catalog.GetQualityMonitorRequest` dataclass. * Added `databricks.sdk.service.catalog.ListConnectionsRequest` dataclass. * Changed `cluster_status()` method for [w.libraries](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/libraries.html) workspace-level service to return `databricks.sdk.service.compute.ClusterLibraryStatuses` dataclass. * Removed `databricks.sdk.service.compute.ClusterStatusResponse` dataclass. * Removed `sort_by_spec` field for `databricks.sdk.service.marketplace.ListListingsRequest`. * Added `is_ascending` field for `databricks.sdk.service.marketplace.ListListingsRequest`. * Added `sort_by` field for `databricks.sdk.service.marketplace.ListListingsRequest`. * Added `is_ascending` field for `databricks.sdk.service.marketplace.SearchListingsRequest`. * Removed `databricks.sdk.service.marketplace.SortBySpec` dataclass. * Removed `databricks.sdk.service.marketplace.SortOrder` dataclass. * Added `gateway_definition` field for `databricks.sdk.service.pipelines.CreatePipeline`. * Added `gateway_definition` field for `databricks.sdk.service.pipelines.EditPipeline`. * Added `table_configuration` field for `databricks.sdk.service.pipelines.ManagedIngestionPipelineDefinition`. * Added `gateway_definition` field for `databricks.sdk.service.pipelines.PipelineSpec`. * Added `table_configuration` field for `databricks.sdk.service.pipelines.SchemaSpec`. * Added `table_configuration` field for `databricks.sdk.service.pipelines.TableSpec`. * Added `databricks.sdk.service.pipelines.IngestionGatewayPipelineDefinition` dataclass. * Added `databricks.sdk.service.pipelines.TableSpecificConfig` dataclass. * Added `databricks.sdk.service.pipelines.TableSpecificConfigScdType` dataclass. * Added `deployment_artifacts` field for `databricks.sdk.service.serving.AppDeployment`. * Added `route_optimized` field for `databricks.sdk.service.serving.CreateServingEndpoint`. * Added `contents` field for `databricks.sdk.service.serving.ExportMetricsResponse`. * Added `endpoint_url` field for `databricks.sdk.service.serving.ServingEndpointDetailed`. * Added `route_optimized` field for `databricks.sdk.service.serving.ServingEndpointDetailed`. * Added `databricks.sdk.service.serving.AppDeploymentArtifacts` dataclass. * Added `scan_index()` method for [w.vector_search_indexes](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/vector_search_indexes.html) workspace-level service. * Added `databricks.sdk.service.vectorsearch.ListValue` dataclass. * Added `databricks.sdk.service.vectorsearch.MapStringValueEntry` dataclass. * Added `databricks.sdk.service.vectorsearch.ScanVectorIndexRequest` dataclass. * Added `databricks.sdk.service.vectorsearch.ScanVectorIndexResponse` dataclass. * Added `databricks.sdk.service.vectorsearch.Struct` dataclass. * Added `databricks.sdk.service.vectorsearch.Value` dataclass. OpenAPI SHA: b3f92dd59b5ee4ab919de4f04ed83c7585c86f2f, Date: 2024-05-16
Hi @sodle-splunk, thanks for raising this, the release with fix has been done: https://github.com/databricks/databricks-sdk-py/releases/tag/v0.27.1 |
Changes
Some delete end points return null instead of empty body, we fix that in SDK Client since the APIs are not available publicly and support isn't available for them yet.
Tests
Thanks to @sodle-splunk for contributing the unit test (#641)
make test
run locallymake fmt
applied