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] Fix parsing issue in ErrorDetail #328

Merged
merged 4 commits into from
Sep 25, 2024

Conversation

costichiulan
Copy link
Contributor

@costichiulan costichiulan commented Aug 6, 2024

Issue Description

Exceptions are not deserialized correctly, the message field contains the response JSON and the string 'Cannot construct instance of com.databricks.sdk.core.error.ErrorDetail, problem: Cannot invoke "Object.getClass()" because "m" is null'

Changes

In ErrorDetails class, i added a null check before the unmodifiableMap is created.

Tests

Created an unit test in ApiErrorBodyDeserializationSuite.java

Signed-off-by: Costin Chiulan costin.chiulan@gmail.com

@costichiulan
Copy link
Contributor Author

@mgyucht

@renaudhartert-db renaudhartert-db self-assigned this Sep 24, 2024
@renaudhartert-db renaudhartert-db changed the title [ISSUE] Error parsing Databricks Error #260 [Fix] Error parsing Databricks Error #260 Sep 24, 2024
@costichiulan costichiulan changed the title [Fix] Error parsing Databricks Error #260 [ISSUE][Fix] Error parsing Databricks Error #260 Sep 24, 2024
@costichiulan costichiulan changed the title [ISSUE][Fix] Error parsing Databricks Error #260 [Fix] Error parsing Databricks Error #260 Sep 24, 2024
@renaudhartert-db
Copy link
Contributor

renaudhartert-db commented Sep 24, 2024

@costichiulan, could you update the title of your PR to [Fix] Fix parsing issue in ErrorDetail so that the change can properly be handled by the changelogs automation? We aim to have our PR titles phrase as imperative sentences.

@costichiulan costichiulan changed the title [Fix] Error parsing Databricks Error #260 [Fix] Fix parsing issue in ErrorDetail Sep 24, 2024
@costichiulan
Copy link
Contributor Author

@costichiulan, could you update the title of your PR to [Fix] Fix parsing issue in ErrorDetail so that the change can properly be handled by the changelogs automation? We aim to have our PR titles phrase as imperative sentences.

Done, pushed also a new commit after executing mvn spotless:apply to pass fmt workflow

@renaudhartert-db
Copy link
Contributor

@costichiulan, could you update the title of your PR to [Fix] Fix parsing issue in ErrorDetail so that the change can properly be handled by the changelogs automation? We aim to have our PR titles phrase as imperative sentences.

Done, pushed also a new commit after executing mvn spotless:apply to pass fmt workflow

Looks good, thanks! I need to double check a couple of things internally before we can proceed with merging the PR but we should be ready to go soon.

@renaudhartert-db
Copy link
Contributor

renaudhartert-db commented Sep 25, 2024

Hi @costichiulan! Sorry for the delay.

There's just one small last step we would need from you before proceeding with the merge.

To contribute to this repository, we need you to sign off your PR to certify that you have the right to contribute the code and that it complies with the open source license. The rules are pretty simple, if you can certify the content of DCO, then simply add a "Signed-off-by" line to the bottom of your PR description message to certify your compliance. Please use your real name as pseudonymous/anonymous contributions are not accepted.

Signed-off-by: Joe Smith <joe.smith@email.com>

Once this is done, I will approve and merge the fix. Thanks again for contributing!

@costichiulan
Copy link
Contributor Author

Hi @costichiulan! Sorry for the delay.

There's just one small last step we would need from you before proceeding with the merge.

To contribute to this repository, we need you to sign off your PR to certify that you have the right to contribute the code and that it complies with the open source license. The rules are pretty simple, if you can certify the content of DCO, then simply add a "Signed-off-by" line to the bottom of your PR description message to certify your compliance. Please use your real name as pseudonymous/anonymous contributions are not accepted.

Signed-off-by: Joe Smith <joe.smith@email.com>

Once this is done, I will approve and merge the fix. Thanks again for contributing!

Done

@renaudhartert-db
Copy link
Contributor

renaudhartert-db commented Sep 25, 2024

@costichiulan, one more thing, our repository requires all commits to be signed for merging. Could you back sign them? Here's a stack-overflow thread that might provide some useful pointers on how to do so.

auto-merge was automatically disabled September 25, 2024 19:56

Head branch was pushed to by a user without write access

@costichiulan
Copy link
Contributor Author

@costichiulan, one more thing, our repository requires all commits to be signed for merging. Could you back sign them? Here's a stack-overflow thread that might provide some useful pointers on how to do so.

Done, thanks for info

@renaudhartert-db renaudhartert-db added this pull request to the merge queue Sep 25, 2024
Merged via the queue into databricks:main with commit 841b346 Sep 25, 2024
10 checks passed
@costichiulan costichiulan deleted the cchiulan/issue-260 branch September 26, 2024 06:46
github-merge-queue bot pushed a commit that referenced this pull request Sep 26, 2024
### Bug Fixes

* Fix listAccountMetastoreAssignments Integration test
([#350](#350))
* Fix parsing issue in ErrorDetail
([#328](#328))

### Internal Changes

* Update SDK to OpenAPI spec
([#346](#346)).
* Add DCO guidelines
([#351](#351))
 

### API Changes:

* Added `workspaceClient.disableLegacyAccess()` service and
`accountClient.disableLegacyFeatures()` service.
 * Added `workspaceClient.temporaryTableCredentials()` service.
* Added `putAiGateway()` method for `workspaceClient.servingEndpoints()`
service.
* Added `com.databricks.sdk.service.apps.ApplicationState`,
`com.databricks.sdk.service.apps.ApplicationStatus`,
`com.databricks.sdk.service.apps.ComputeState` and
`com.databricks.sdk.service.apps.ComputeStatus` classes.
* Added `com.databricks.sdk.service.catalog.AwsCredentials`,
`com.databricks.sdk.service.catalog.AzureUserDelegationSas`,
`com.databricks.sdk.service.catalog.GcpOauthToken`,
`com.databricks.sdk.service.catalog.GenerateTemporaryTableCredentialRequest`,
`com.databricks.sdk.service.catalog.GenerateTemporaryTableCredentialResponse`,
`com.databricks.sdk.service.catalog.R2Credentials` and
`com.databricks.sdk.service.catalog.TableOperation` classes.
* Added `com.databricks.sdk.service.serving.AiGatewayConfig`,
`com.databricks.sdk.service.serving.AiGatewayGuardrailParameters`,
`com.databricks.sdk.service.serving.AiGatewayGuardrailPiiBehavior`,
`com.databricks.sdk.service.serving.AiGatewayGuardrailPiiBehaviorBehavior`,
`com.databricks.sdk.service.serving.AiGatewayGuardrails`,
`com.databricks.sdk.service.serving.AiGatewayInferenceTableConfig`,
`com.databricks.sdk.service.serving.AiGatewayRateLimit`,
`com.databricks.sdk.service.serving.AiGatewayRateLimitKey`,
`com.databricks.sdk.service.serving.AiGatewayRateLimitRenewalPeriod`,
`com.databricks.sdk.service.serving.AiGatewayUsageTrackingConfig`,
`com.databricks.sdk.service.serving.PutAiGatewayRequest` and
`com.databricks.sdk.service.serving.PutAiGatewayResponse` classes.
* Added `com.databricks.sdk.service.settings.BooleanMessage`,
`com.databricks.sdk.service.settings.DeleteDisableLegacyAccessRequest`,
`com.databricks.sdk.service.settings.DeleteDisableLegacyAccessResponse`,
`com.databricks.sdk.service.settings.DeleteDisableLegacyFeaturesRequest`,
`com.databricks.sdk.service.settings.DeleteDisableLegacyFeaturesResponse`,
`com.databricks.sdk.service.settings.DisableLegacyAccess`,
`com.databricks.sdk.service.settings.DisableLegacyFeatures`,
`com.databricks.sdk.service.settings.GetDisableLegacyAccessRequest`,
`com.databricks.sdk.service.settings.GetDisableLegacyFeaturesRequest`,
`com.databricks.sdk.service.settings.UpdateDisableLegacyAccessRequest`
and
`com.databricks.sdk.service.settings.UpdateDisableLegacyFeaturesRequest`
classes.
* Added `appStatus` and `computeStatus` fields for
`com.databricks.sdk.service.apps.App`.
* Added `deploymentId` field for
`com.databricks.sdk.service.apps.CreateAppDeploymentRequest`.
* Added `externalAccessEnabled` field for
`com.databricks.sdk.service.catalog.GetMetastoreSummaryResponse`.
* Added `includeManifestCapabilities` field for
`com.databricks.sdk.service.catalog.GetTableRequest`.
* Added `includeManifestCapabilities` field for
`com.databricks.sdk.service.catalog.ListSummariesRequest`.
* Added `includeManifestCapabilities` field for
`com.databricks.sdk.service.catalog.ListTablesRequest`.
* Added `externalAccessEnabled` field for
`com.databricks.sdk.service.catalog.MetastoreInfo`.
* Added `budgetPolicyId` field for
`com.databricks.sdk.service.pipelines.CreatePipeline`.
* Added `budgetPolicyId` field for
`com.databricks.sdk.service.pipelines.EditPipeline`.
* Added `effectiveBudgetPolicyId` field for
`com.databricks.sdk.service.pipelines.GetPipelineResponse`.
* Added `budgetPolicyId` field for
`com.databricks.sdk.service.pipelines.PipelineSpec`.
* Added `aiGateway` field for
`com.databricks.sdk.service.serving.CreateServingEndpoint`.
* Added `aiGateway` field for
`com.databricks.sdk.service.serving.ServingEndpoint`.
* Added `aiGateway` field for
`com.databricks.sdk.service.serving.ServingEndpointDetailed`.
* Added `workspaceId` field for
`com.databricks.sdk.service.settings.TokenInfo`.
* Changed `delete()`, `start()` and `stop()` methods for
`workspaceClient.apps()` service to return
`com.databricks.sdk.service.apps.App` class.
* Changed `deploy()` method for `workspaceClient.apps()` service with
new required argument order.
* Changed `sourceCodePath` field for
`com.databricks.sdk.service.apps.AppDeployment` to no longer be
required.
* Changed `sourceCodePath` field for
`com.databricks.sdk.service.apps.CreateAppDeploymentRequest` to no
longer be required.
* Changed `returnParams` and `routineDependencies` fields for
`com.databricks.sdk.service.catalog.CreateFunction` to no longer be
required.
* Removed `com.databricks.sdk.service.apps.AppState`,
`com.databricks.sdk.service.apps.AppStatus`, `Object` and `Object`
classes.
* Removed `com.databricks.sdk.service.sql.ClientCallContext`,
`com.databricks.sdk.service.sql.EncodedText`,
`com.databricks.sdk.service.sql.EncodedTextEncoding`,
`com.databricks.sdk.service.sql.QuerySource`,
`com.databricks.sdk.service.sql.QuerySourceDriverInfo`,
`com.databricks.sdk.service.sql.QuerySourceEntryPoint`,
`com.databricks.sdk.service.sql.QuerySourceJobManager`,
`com.databricks.sdk.service.sql.QuerySourceTrigger` and
`com.databricks.sdk.service.sql.ServerlessChannelInfo` classes.
 * Removed `status` field for `com.databricks.sdk.service.apps.App`.
* Removed `querySource` field for
`com.databricks.sdk.service.sql.QueryInfo`.

OpenAPI SHA: 6f6b1371e640f2dfeba72d365ac566368656f6b6, Date: 2024-09-19
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