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 compatibility with Kafka >= 3.8 #194

Merged
merged 1 commit into from
Aug 5, 2024

Conversation

juha-aiven
Copy link
Contributor

@juha-aiven juha-aiven commented Aug 5, 2024

Stop using io.aiven.kafka.auth.audit.Session from Kafka that is no longer present in Kafka >= 3.8.

Add io.aiven.kafka.auth.audit.Session to replace that. In the new class, add properly named getter methods and make the fields private.

Fixes #193.

@juha-aiven juha-aiven force-pushed the juham/fix-session branch 2 times, most recently from 10707fd to 2456272 Compare August 5, 2024 06:41
Stop using io.aiven.kafka.auth.audit.Session from Kafka that is no longer present in Kafka >= 3.8.

Add io.aiven.kafka.auth.audit.Session to replace that. In the new class, add properly named getter methods and make the fields private.
@juha-aiven juha-aiven marked this pull request as ready for review August 5, 2024 06:51
@juha-aiven juha-aiven requested review from a team as code owners August 5, 2024 06:51
Copy link
Contributor

@jlprat jlprat left a comment

Choose a reason for hiding this comment

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

LGTM Thanks @juha-aiven

Copy link
Contributor

@jeqo jeqo left a comment

Choose a reason for hiding this comment

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

add properly named getter methods

nit: nothing wrong on not having get prefixed methods anymore. For instance, Java Records come with accessor methods named same as attributes

For each component in the header, two members: a public accessor method with the same name and return type as the component, and a private final field with the same type as the component;

@jlprat
Copy link
Contributor

jlprat commented Aug 5, 2024

Regarding the getter setter. It all boils down to when did you get POJOs best practices imprinted in your mind.
This is a class that could definitely be replaced with a Java record as it was a case class in Scala.
But, in the end, this looks like something that can be decided project wise and chose a "policy" for POJOs and style guide for some type of stylistic questions.

@jeqo jeqo merged commit e4dfb72 into Aiven-Open:main Aug 5, 2024
4 checks passed
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.

Incompatible with Kafka >= 3.8
3 participants