Skip to content

Added jvm codecs implementations #414

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

Merged
merged 1 commit into from
Jul 30, 2025
Merged

Added jvm codecs implementations #414

merged 1 commit into from
Jul 30, 2025

Conversation

Mr3zee
Copy link
Collaborator

@Mr3zee Mr3zee commented Jul 29, 2025

Subsystem
gRPC

Problem Description
We had codecs for K/N but not for jvm

Solution
Add them for jvm (and fix some issues along the way)

@Mr3zee Mr3zee requested a review from Jozott00 July 29, 2025 19:41
@Mr3zee Mr3zee self-assigned this Jul 29, 2025
@Mr3zee Mr3zee added the feature New feature or request label Jul 29, 2025
Copy link
Collaborator

@Jozott00 Jozott00 left a comment

Choose a reason for hiding this comment

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

Looks very good to me. Just the two cases where you could avoid coping when reading/writing byte arrays.

Comment on lines +15 to +18
// errors in jvm are exceptions
override fun hadError(): Boolean {
return false
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should probably unify the behavior between all implementations. Probably, the K/N implementation should also throw exceptions instead of providing the error indicator? (in a separate PR)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, let's do that in the next PR

@Mr3zee Mr3zee merged commit 75f8b3c into grpc-common Jul 30, 2025
2 of 3 checks passed
@Mr3zee Mr3zee deleted the grpc-jvm/codec branch July 30, 2025 12:00
Mr3zee added a commit that referenced this pull request Aug 1, 2025
Mr3zee added a commit that referenced this pull request Aug 4, 2025
Mr3zee added a commit that referenced this pull request Aug 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants