Skip to content

KAFKA-14737: Move kafka.utils.json to server-common#13585

Merged
mimaison merged 1 commit intoapache:trunkfrom
OmniaGM:KAFKA-14737
Jul 18, 2023
Merged

KAFKA-14737: Move kafka.utils.json to server-common#13585
mimaison merged 1 commit intoapache:trunkfrom
OmniaGM:KAFKA-14737

Conversation

@OmniaGM
Copy link
Copy Markdown
Contributor

@OmniaGM OmniaGM commented Apr 16, 2023

This is a replica of kafka.utils.json, we can decide to switch to org.apache.kafka.server.util.json everywhere later.

More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.

Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@OmniaGM OmniaGM force-pushed the KAFKA-14737 branch 6 times, most recently from f16a1cd to 0f083d9 Compare April 21, 2023 11:55
@OmniaGM OmniaGM changed the title [WIP] KAFKA-14737: Move kafka.utils.json to server-common KAFKA-14737: Move kafka.utils.json to server-common Apr 21, 2023
@fvaleri fvaleri added the tools label Jul 7, 2023
Copy link
Copy Markdown
Contributor

@fvaleri fvaleri left a comment

Choose a reason for hiding this comment

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

LGTM. Nice work.

Can you please fix the checkstyle import errors?

@OmniaGM
Copy link
Copy Markdown
Contributor Author

OmniaGM commented Jul 9, 2023

LGTM. Nice work.

Can you please fix the checkstyle import errors?

Thanks for the review. I fixed the import errors

@fvaleri fvaleri requested a review from showuon July 10, 2023 14:59
Copy link
Copy Markdown
Member

@mimaison mimaison 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 the PR! I left one question.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need this catch is we rethrow the same exception directly?
Same in decodeMap() below

Copy link
Copy Markdown
Contributor Author

@OmniaGM OmniaGM Jul 12, 2023

Choose a reason for hiding this comment

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

Probably not, I was trying to stick to the original interface (which doesn't have any exception in the signature) as much as I could in case we switched everything to use this new class instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I removed the try/catch

@OmniaGM OmniaGM force-pushed the KAFKA-14737 branch 2 times, most recently from 3631fcf to b13a2b9 Compare July 13, 2023 13:45
Copy link
Copy Markdown
Member

@mimaison mimaison 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!

@mimaison mimaison merged commit 0c6b1a4 into apache:trunk Jul 18, 2023
jolshan added a commit to confluentinc/kafka that referenced this pull request Jul 18, 2023
* ak/trunk: (110 commits)
  MINOR: Update docs to include ZK deprecation notice and information (apache#14031)
  KAFKA-15091: Fix misleading Javadoc for SourceTask::commit (apache#13948)
  KAFKA-14669: Use the generated docs for MirrorMaker configs in the doc (apache#13658)
  KAFKA-14953: Add tiered storage related metrics (apache#13944)
  KAFKA-15121: Implement the alterOffsets method in the FileStreamSourceConnector and the FileStreamSinkConnector (apache#13945)
  Revert "MINOR: Update .asf.yaml file with refreshed github_whitelist, and collaborators" (apache#14037)
  MINOR: Update .asf.yaml file with refreshed github_whitelist, and collaborators
  KAFKA-14737: Move kafka.utils.json to server-common (apache#13585)
  KAFKA-14647: Move TopicFilter to server-common/utils (apache#13158)
  MINOR: remove unused variable in examples (apache#14021)
  ...
jeqo pushed a commit to jeqo/kafka that referenced this pull request Jul 21, 2023
Reviewers: Mickael Maison <mickael.maison@gmail.com>, Federico Valeri <fedevaleri@gmail.com>
Cerchie pushed a commit to Cerchie/kafka that referenced this pull request Jul 25, 2023
Reviewers: Mickael Maison <mickael.maison@gmail.com>, Federico Valeri <fedevaleri@gmail.com>
jeqo pushed a commit to aiven/kafka that referenced this pull request Aug 15, 2023
Reviewers: Mickael Maison <mickael.maison@gmail.com>, Federico Valeri <fedevaleri@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants