kafka icon indicating copy to clipboard operation
kafka copied to clipboard
verified publisher icon apache

KAFKA-16666: Migrate `TransactionLogMessageFormatter` to tools module

Open m1a2st opened this issue 1 year ago • 14 comments

These class are used by console consumer. Now I want to migrate there code from Scala to Java

m1a2st avatar May 21 '24 15:05 m1a2st

@m1a2st could you please fix the conflicts

chia7712 avatar May 22 '24 14:05 chia7712

@chia7712, Please take a look, Thank you

m1a2st avatar Jun 04 '24 13:06 m1a2st

This PR includes many migration, and hence it gets a bit complicated. Maybe we should separate it into different PRs. For example:

  1. move TransactionLog to transaction-coordinator module. this include TransactionLogMessageFormatter migration
  2. move OffsetsMessageFormatter
  3. move GroupMetadataMessageFormatter

WDYT?

chia7712 avatar Jun 15 '24 04:06 chia7712

I think that seperate to three PRs are good, these formatter are not dependency for each other.

m1a2st avatar Jun 15 '24 05:06 m1a2st

just have another idea for minimizing the changes: Maybe we don't touch the code in the core, but we deprecate them. And then we add new message formatter instead. For example:

  1. add new TransactionLogMessageFormatter and deprecate core code
  2. add new OffsetsMessageFormatter and deprecate core code
  3. add new GroupMetadataMessageFormatter and deprecate core code

chia7712 avatar Jun 16 '24 00:06 chia7712

If we mark core Formatter to deprecate, should It need to delete at next Kafka version?

m1a2st avatar Jun 16 '24 05:06 m1a2st

If we mark core Formatter to deprecate, should It need to delete at next Kafka version?

yep if we push the code to deprecate them in 3.9.0

chia7712 avatar Jun 16 '24 07:06 chia7712

@chia7712, Thanks for your comments. I change the checkstyle doc.

m1a2st avatar Jun 22 '24 05:06 m1a2st

@m1a2st please fix the spotless error

chia7712 avatar Jun 24 '24 15:06 chia7712

@chia7712, Thanks for your comments, FIx it

m1a2st avatar Jun 24 '24 15:06 m1a2st

@chia7712 gentle ping, PTAL

m1a2st avatar Jun 27 '24 14:06 m1a2st

@chia7712, Thanks for your patient, PTAL

m1a2st avatar Jun 28 '24 08:06 m1a2st

@m1a2st could you please rebase code to trigger qa ?

chia7712 avatar Jul 01 '24 11:07 chia7712

Thank you @chia7712 for the reminder, rebased.

m1a2st avatar Jul 01 '24 11:07 m1a2st

@m1a2st could you please rebase code ?

chia7712 avatar Jul 05 '24 17:07 chia7712

Thank you @chia7712 for the reminder, rebased.

m1a2st avatar Jul 07 '24 01:07 m1a2st

@m1a2st please take a look at build error

chia7712 avatar Jul 08 '24 00:07 chia7712

@chia7712, Thanks for your comments, PTAL

m1a2st avatar Jul 10 '24 12:07 m1a2st

@chia7712, Thanks for your comments, add new transction test, PTAL

m1a2st avatar Jul 13 '24 03:07 m1a2st

@chia7712, Thanks for your comments, PTAL

m1a2st avatar Jul 17 '24 13:07 m1a2st

@m1a2st pleae fix the build error

chia7712 avatar Jul 18 '24 03:07 chia7712

@chia7712, PTAL, Thanks

m1a2st avatar Jul 19 '24 14:07 m1a2st

@chia7712, Thanks for your comments, PTAL

m1a2st avatar Jul 22 '24 09:07 m1a2st

Hi @chia7712 @m1a2st this commit is causing deterministic test failures on trunk: https://issues.apache.org/jira/browse/KAFKA-17195 PTAL, thanks!

gharris1727 avatar Jul 24 '24 23:07 gharris1727

@gharris1727, Thanks for your comments, I will take a look and fix it.

m1a2st avatar Jul 25 '24 00:07 m1a2st

@gharris1727 thanks for kind reminder. I will check it asap

chia7712 avatar Jul 25 '24 03:07 chia7712