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

THRIFT-5833: Add ProcessorError #3063

Merged
merged 1 commit into from
Nov 15, 2024
Merged

THRIFT-5833: Add ProcessorError #3063

merged 1 commit into from
Nov 15, 2024

Conversation

fishy
Copy link
Member

@fishy fishy commented Nov 14, 2024

Client: go

Provide ProcessorError as the combined error of write i/o error and original error returned by the endpoint implementation, and update compiler to use that in Process functions.

  • Did you create an Apache Jira ticket? (Request account here, not required for trivial changes)
  • If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
  • Did you squash your changes to a single commit? (not required, but preferred)
  • Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
  • If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources.

@fishy fishy force-pushed the go-process-combined-error branch from 86f2045 to eb48ad8 Compare November 15, 2024 00:20
@fishy fishy added the golang patches related to go language label Nov 15, 2024
@fishy fishy requested a review from dcelasun November 15, 2024 00:25
@fishy fishy marked this pull request as ready for review November 15, 2024 00:25
Copy link
Member

@dcelasun dcelasun left a comment

Choose a reason for hiding this comment

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

LGTM

// TProcessorError is the combined original error returned by the endpoint
// implementation, and I/O error when writing the response back to the client.
//
// This type will be returned by Process function if there's an error happend
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// This type will be returned by Process function if there's an error happend
// This type will be returned by Process function if there's an error happened

@fishy fishy force-pushed the go-process-combined-error branch from eb48ad8 to f29a1e4 Compare November 15, 2024 16:43
Client: go

Provide ProcessorError as the combined error of write i/o error and
original error returned by the endpoint implementation, and update
compiler to use that in Process functions.
@fishy fishy force-pushed the go-process-combined-error branch from f29a1e4 to 01c134d Compare November 15, 2024 17:35
@fishy fishy changed the title THRIFT-5833: Add TProcessorError THRIFT-5833: Add ProcessorError Nov 15, 2024
@fishy
Copy link
Member Author

fishy commented Nov 15, 2024

@dcelasun I renamed it from TProcessorError to ProcessorError. I think we use the T- prefix for things in the spec and shared by most languages, and this is a go only thing. PTAL.

@fishy fishy requested a review from dcelasun November 15, 2024 17:36
@fishy fishy merged commit c8cf304 into master Nov 15, 2024
37 of 42 checks passed
@fishy fishy deleted the go-process-combined-error branch November 15, 2024 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
golang patches related to go language
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants