Skip to content

MINOR: Remove casts to KRaftMetadataCache#18579

Merged
ijuma merged 1 commit intoapache:trunkfrom
ijuma:remove-kraft-metadata-cache-casts
Jan 17, 2025
Merged

MINOR: Remove casts to KRaftMetadataCache#18579
ijuma merged 1 commit intoapache:trunkfrom
ijuma:remove-kraft-metadata-cache-casts

Conversation

@ijuma
Copy link
Copy Markdown
Member

@ijuma ijuma commented Jan 16, 2025 •
edited
Loading

Committer Checklist (excluded from commit message)

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

@github-actions github-actions Bot added the core Kafka Broker label Jan 16, 2025
@ijuma ijuma changed the title KAFKA-18373: Remove ZkMetadataCache Remove casts to KRaftMetadataCache Jan 16, 2025
@ijuma ijuma changed the title Remove casts to KRaftMetadataCache MINOR: Remove casts to KRaftMetadataCache Jan 16, 2025
@ijuma ijuma force-pushed the remove-kraft-metadata-cache-casts branch from 4bd584e to acd0980 Compare January 17, 2025 12:48
@github-actions github-actions Bot added the small Small PRs label Jan 17, 2025
@ijuma ijuma force-pushed the remove-kraft-metadata-cache-casts branch from acd0980 to abc37bd Compare January 17, 2025 13:04
@ijuma ijuma marked this pull request as ready for review January 17, 2025 13:05
@ijuma ijuma requested a review from mumrah January 17, 2025 13:05
mimaison
mimaison approved these changes Jan 17, 2025
View reviewed changes
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

@ijuma ijuma merged commit 3996e90 into apache:trunk Jan 17, 2025
@chia7712
Copy link
Copy Markdown
Member

chia7712 commented Jan 19, 2025

@ijuma I'm curious as to why this PR doesn't remove the casting from Partition.scala?

https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/cluster/Partition.scala#L1060
https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/cluster/Partition.scala#L1815

@ijuma
Copy link
Copy Markdown
Member Author

ijuma commented Jan 19, 2025 •
edited
Loading

I looked for casts vs pattern matching. I agree we should also clean up the latter. For your second example, not sure why it didn't show up in the search I used.

@chia7712
Copy link
Copy Markdown
Member

chia7712 commented Jan 19, 2025

I looked for casts vs pattern matching. I agree we should also clean up the latter. For your second example, not sure why it didn't show up in the search I used.

Thank you for your quick response. I was wondering if there was a specific reason for keeping them. @FrankYang0529 will cleanup it in https://issues.apache.org/jira/browse/KAFKA-18555

pranavt84 pushed a commit to pranavt84/kafka that referenced this pull request Jan 27, 2025
@ijuma @pranavt84
Remove casts to KRaftMetadataCache (apache#18579)
890fa56 
Reviewers: Mickael Maison <mickael.maison@gmail.com>
airlock-confluentinc Bot pushed a commit to confluentinc/kafka that referenced this pull request Jan 27, 2025
@ijuma @mjsax
Remove casts to KRaftMetadataCache (apache#18579)
c4bad98 
Reviewers: Mickael Maison <mickael.maison@gmail.com>
manoj-mathivanan pushed a commit to manoj-mathivanan/kafka that referenced this pull request Feb 19, 2025
@ijuma @manoj-mathivanan
Remove casts to KRaftMetadataCache (apache#18579)
5eb81f3 
Reviewers: Mickael Maison <mickael.maison@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mimaison mimaison mimaison approved these changes

@mumrah mumrah Awaiting requested review from mumrah

Assignees

No one assigned

Labels

core Kafka Broker small Small PRs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ijuma @chia7712 @mimaison