Skip to content

Fix serialized name for retriable QueryError property#19741

Merged
tdcmeehan merged 1 commit intoprestodb:masterfrom
MasterOdin:retriable-property
Dec 21, 2023
Merged

Fix serialized name for retriable QueryError property#19741
tdcmeehan merged 1 commit intoprestodb:masterfrom
MasterOdin:retriable-property

Conversation

@MasterOdin
Copy link
Contributor

Test plan - Added unit test in presto-client/src/test/java/com/facebook/presto/client/TestQueryError.java

== RELEASE NOTES ==

General Changes
* Fix serialized name for retriable QueryError property

@MasterOdin MasterOdin requested a review from a team as a code owner May 26, 2023 12:32
@MasterOdin MasterOdin requested a review from ajaygeorge May 26, 2023 12:32
Copy link
Contributor

@tdcmeehan tdcmeehan left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

@tdcmeehan
Copy link
Contributor

Please rebase to kick off latest tests. I'll merge when finished.

@MasterOdin
Copy link
Contributor Author

@tdcmeehan Done!

@MasterOdin
Copy link
Contributor Author

@tdcmeehan I've rebased again. Looking at the test failures, I'm not sure why they would fail, as they appear to be unrelated to my change.

@tdcmeehan tdcmeehan self-assigned this Dec 21, 2023
Fixes a bug where the retriable property for QueryError was
being serialized to the string "boolean".
@tdcmeehan
Copy link
Contributor

Rebased again, should fix the errors. Will merge once tests pass.

Copy link
Contributor

@ajaygeorge ajaygeorge left a comment

Choose a reason for hiding this comment

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

LGTM

@tdcmeehan tdcmeehan merged commit 8aebd34 into prestodb:master Dec 21, 2023
@tdcmeehan
Copy link
Contributor

Thanks once again @MasterOdin

@MasterOdin MasterOdin deleted the retriable-property branch December 21, 2023 18:25
@wanglinsong wanglinsong mentioned this pull request Feb 12, 2024
64 tasks
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.

4 participants