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

Fix null body response to empty in ApiClient #579

Merged
merged 12 commits into from
May 16, 2024
Merged

Fix null body response to empty in ApiClient #579

merged 12 commits into from
May 16, 2024

Conversation

tanmay-db
Copy link
Contributor

@tanmay-db tanmay-db commented Mar 7, 2024

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 locally
  • make fmt applied
  • relevant integration tests applied

Copy link
Contributor

@mgyucht mgyucht left a comment

Choose a reason for hiding this comment

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

Test?

@codecov-commenter
Copy link

codecov-commenter commented Mar 7, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 57.62%. Comparing base (e10bd59) to head (057bf5a).

Files Patch % Lines
databricks/sdk/core.py 83.33% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

@tanmay-db
Copy link
Contributor Author

Test?

@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.

Copy link

github-actions bot commented May 15, 2024

This PR breaks backwards compatibility for databrickslabs/blueprint downstream. See build logs for more details.

Running from downstreams #108

@tanmay-db
Copy link
Contributor Author

Note: The failing test: downstreams / compatibility (blueprint, databrickslabs) (pull_request) fails on a simple text change: #643. This passed on last commit which was 2 weeks ago and doesn't seem to be happening due to changes in Python SDK.

@tanmay-db tanmay-db requested a review from mgyucht May 15, 2024 13:49
@nkvuong
Copy link
Contributor

nkvuong commented May 15, 2024

the failed check is not related to this change, the same test is failing on blueprint/main
image

Copy link
Contributor

@sodle-splunk sodle-splunk left a 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. 🥳

@mgyucht mgyucht added this pull request to the merge queue May 16, 2024
Merged via the queue into main with commit 53e99a5 May 16, 2024
8 of 9 checks passed
@mgyucht mgyucht deleted the fix-ucx branch May 16, 2024 09:55
tanmay-db added a commit that referenced this pull request May 16, 2024
* 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
@tanmay-db tanmay-db mentioned this pull request May 16, 2024
tanmay-db added a commit that referenced this pull request May 16, 2024
* Fix null body response to empty in ApiClient ([#579](#579)).
@tanmay-db tanmay-db mentioned this pull request May 16, 2024
github-merge-queue bot pushed a commit that referenced this pull request May 16, 2024
## 0.27.1

### Bug Fixes
* Fix null body response to empty in ApiClient
([#579](#579)).

Note: This is a patch release. The release with SDK Generation will be
done as part of biweekly cadence.
@tanmay-db
Copy link
Contributor Author

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

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.

None yet

5 participants