Override INVALID_PARAMETER_VALUE on fetching non-existent job/cluster #864

mgyucht · 2024-03-22T15:34:34Z

Changes

Most services use RESOURCE_DOES_NOT_EXIST error code with 404 status code to indicate that a resource doesn't exist. However, for legacy reasons, Jobs and Clusters services use INVALID_PARAMETER_VALUE error code with 400 status code instead. This makes tools like Terraform and UCX difficult to maintain, as these services need different error handling logic. However, we can't change these behaviors as customers already depend on the raw HTTP response status & contents.

This PR corrects these errors in the SDK itself. SDK users can then do

_, err := w.Jobs.Get(ctx, jobs.GetJobRequest{JobId: 12345})
if errors.Is(err, apierr.ErrResourceDoesNotExist) { ... }

just as you would expect from other resources.

Updated the README with more information about this as well.

Tests

Added unit tests for error overrides.
Added 2 integration tests for Clusters and Jobs.

make test passing
make fmt applied
relevant integration tests applied

codecov-commenter · 2024-03-22T15:57:03Z

Codecov Report

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

Project coverage is 8.04%. Comparing base (a823ca3) to head (8c89e52).

Files	Patch %	Lines
apierr/error_override.go	71.42%	4 Missing and 2 partials ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##            main    #864      +/-   ##
========================================
+ Coverage   8.00%   8.04%   +0.03%     
========================================
  Files        258     259       +1     
  Lines      64416   64441      +25     
========================================
+ Hits        5155    5182      +27     
+ Misses     58966   58959       -7     
- Partials     295     300       +5

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

tanmay-db

LGTM

tanmay-db · 2024-03-22T20:00:41Z

apierr/error_override.go

+	return true
+}
+
+var invalidParameterValue = "INVALID_PARAMETER_VALUE"


If we aren't changing this, then it would be good to have this as a constant

tanmay-db · 2024-03-22T20:03:40Z

apierr/error_override.go

+
+var invalidParameterValue = "INVALID_PARAMETER_VALUE"
+
+var allOverrides = []errorOverride{


nit: Might be good to encapsulate this in a method so it's not accidentally modified.

tanmay-db · 2024-03-22T20:06:03Z

apierr/error_override.go

+		pathRegex:        regexp.MustCompile(`^/api/2\.0/clusters/get`),
+		verb:             "GET",
+		messageMatcher:   regexp.MustCompile("Cluster .* does not exist"),
+		errorCodeMatcher: regexp.MustCompile(invalidParameterValue),


Should we have a statusCodeMatcher?

Good idea, will add.

…#591) ## Changes Port of databricks/databricks-sdk-go#864 to the Python SDK. Most services use RESOURCE_DOES_NOT_EXIST error code with 404 status code to indicate that a resource doesn't exist. However, for legacy reasons, Jobs and Clusters services use INVALID_PARAMETER_VALUE error code with 400 status code instead. This makes tools like Terraform and UCX difficult to maintain, as these services need different error handling logic. However, we can't change these behaviors as customers already depend on the raw HTTP response status & contents. This PR corrects these errors in the SDK itself. SDK users can then do ``` try: w.jobs.get_by_id('abc') except ResourceDoesNotExist: pass ``` just as you would expect from other resources. Updated the README with more information about this as well. ## Tests  - [ ] `make test` run locally - [ ] `make fmt` applied - [ ] relevant integration tests applied

…#257) ## Changes Ports databricks/databricks-sdk-go#864 to the Java SDK. Most services use `RESOURCE_DOES_NOT_EXIST` error code with 404 status code to indicate that a resource doesn't exist. However, for legacy reasons, Jobs and Clusters services use `INVALID_PARAMETER_VALUE` error code with 400 status code instead. This makes tools like Terraform and UCX difficult to maintain, as these services need different error handling logic. However, we can't change these behaviors as customers already depend on the raw HTTP response status & contents. This PR corrects these errors in the SDK itself. SDK users can then do ```java try { BaseJob job = w.jobs().get("123"); } catch (ResourceDoesNotExist e) { ... } ``` just as you would expect from other resources. Updated the README with more information about this as well. ## Tests Added unit tests for error overrides. Added/updated the integration tests for Clusters and Jobs. - [x] `make test` passing - [x] `make fmt` applied - [x] relevant integration tests applied

…databricks#257) ## Changes Ports databricks/databricks-sdk-go#864 to the Java SDK. Most services use `RESOURCE_DOES_NOT_EXIST` error code with 404 status code to indicate that a resource doesn't exist. However, for legacy reasons, Jobs and Clusters services use `INVALID_PARAMETER_VALUE` error code with 400 status code instead. This makes tools like Terraform and UCX difficult to maintain, as these services need different error handling logic. However, we can't change these behaviors as customers already depend on the raw HTTP response status & contents. This PR corrects these errors in the SDK itself. SDK users can then do ```java try { BaseJob job = w.jobs().get("123"); } catch (ResourceDoesNotExist e) { ... } ``` just as you would expect from other resources. Updated the README with more information about this as well. ## Tests Added unit tests for error overrides. Added/updated the integration tests for Clusters and Jobs. - [x] `make test` passing - [x] `make fmt` applied - [x] relevant integration tests applied

mgyucht added 3 commits March 22, 2024 16:02

Override INVALID_PARAMETER_VALUE on fetching non-existent job/cluster

e01c063

bit simpler

305cd9a

docs

c7f55ec

mgyucht requested a review from tanmay-db March 22, 2024 15:34

mgyucht added 2 commits March 22, 2024 16:47

unused

953605c

fix test

3a0d7cd

mgyucht added 2 commits March 22, 2024 18:24

links

9d212aa

another case

8c89e52

mgyucht mentioned this pull request Mar 22, 2024

Override INVALID_PARAMETER_VALUE on fetching non-existent job/cluster databricks/databricks-sdk-py#591

Merged

3 tasks

tanmay-db approved these changes Mar 22, 2024

View reviewed changes

mgyucht added 7 commits March 28, 2024 14:49

Merge branch 'main' into overrides-for-clusters-jobs

7fbc762

work

6a16a61

fix

70c8d47

work

625e795

work

2598082

work

5f09580

fix test

5877184

mgyucht requested a review from tanmay-db March 28, 2024 14:47

use errors.Is when checking IsMissing or TooManyRequests

bba50a1

tanmay-db approved these changes Apr 2, 2024

View reviewed changes

mgyucht added this pull request to the merge queue Apr 3, 2024

Merged via the queue into main with commit a4944ab Apr 3, 2024
5 checks passed

mgyucht deleted the overrides-for-clusters-jobs branch April 3, 2024 11:02

mgyucht mentioned this pull request Apr 4, 2024

Override INVALID_PARAMETER_VALUE on fetching non-existent job/cluster databricks/databricks-sdk-java#257

Merged

3 tasks

mgyucht mentioned this pull request Apr 11, 2024

Release v0.38.0 #889

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Override INVALID_PARAMETER_VALUE on fetching non-existent job/cluster #864

Override INVALID_PARAMETER_VALUE on fetching non-existent job/cluster #864

mgyucht commented Mar 22, 2024

codecov-commenter commented Mar 22, 2024 •

edited

Loading

tanmay-db left a comment

tanmay-db Mar 22, 2024

tanmay-db Mar 22, 2024

tanmay-db Mar 22, 2024

mgyucht Mar 28, 2024


		var invalidParameterValue = "INVALID_PARAMETER_VALUE"

		var allOverrides = []errorOverride{

Override INVALID_PARAMETER_VALUE on fetching non-existent job/cluster #864

Override INVALID_PARAMETER_VALUE on fetching non-existent job/cluster #864

Conversation

mgyucht commented Mar 22, 2024

Changes

Tests

codecov-commenter commented Mar 22, 2024 • edited Loading

Codecov Report

tanmay-db left a comment

Choose a reason for hiding this comment

tanmay-db Mar 22, 2024

Choose a reason for hiding this comment

tanmay-db Mar 22, 2024

Choose a reason for hiding this comment

tanmay-db Mar 22, 2024

Choose a reason for hiding this comment

mgyucht Mar 28, 2024

Choose a reason for hiding this comment

codecov-commenter commented Mar 22, 2024 •

edited

Loading