Skip to content

KAFKA-18539: Remove optional managers in KafkaApis#18550

Merged
chia7712 merged 6 commits intoapache:trunkfrom
apoorvmittal10:KAFKA-18539
Jan 15, 2025
Merged

KAFKA-18539: Remove optional managers in KafkaApis#18550
chia7712 merged 6 commits intoapache:trunkfrom
apoorvmittal10:KAFKA-18539

Conversation

@apoorvmittal10
Copy link
Copy Markdown
Contributor

@apoorvmittal10 apoorvmittal10 commented Jan 15, 2025

Removed Optional for SharePartitionManager and ClientMetricsManager as zookeeper code is being removed. Also removed asScala and asJava conversion in KafkaApis.handleListClientMetricsResources, moved to java stream.

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 triage PRs from the community core Kafka Broker performance labels Jan 15, 2025
Copy link
Copy Markdown
Member

@ijuma ijuma 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.

@chia7712
Copy link
Copy Markdown
Member

@apoorvmittal10 sorry that my recent merge causes some conflicts on this PR. Could you please fix them?

@apoorvmittal10
Copy link
Copy Markdown
Contributor Author

@apoorvmittal10 sorry that my recent merge causes some conflicts on this PR. Could you please fix them?

I rebased.

requestHelper.sendMaybeThrottle(request, listClientMetricsResourcesRequest.getErrorResponse(Errors.UNSUPPORTED_VERSION.exception))
}
val data = new ListClientMetricsResourcesResponseData().setClientMetricsResources(
clientMetricsManager.listClientMetricsResources.asScala.map(
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.

It looks like we could avoid the asScala -> asJava here

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.

Moved to stream and collector, removed asScala and asJava conversions.

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.

Let's mention this change in the PR description since it's unrelated to the core change.

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.

Done.

Copy link
Copy Markdown
Member

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

LGTM

}
val data = new ListClientMetricsResourcesResponseData().setClientMetricsResources(
clientMetricsManager.listClientMetricsResources.stream.map(
name => new ClientMetricsResource().setName(name)).collect(Collectors.toList()))
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.

nit: collect(Collectors.toList()) -> toList

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.

Done.

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

@apoorvmittal10
Copy link
Copy Markdown
Contributor Author

Build passed on Java 23, unrelated test failure on Java 17.

@chia7712 chia7712 merged commit 3fa9984 into apache:trunk Jan 15, 2025
chia7712 pushed a commit that referenced this pull request Jan 15, 2025
Removed Optional for SharePartitionManager and ClientMetricsManager as zookeeper code is being removed. Also removed asScala and asJava conversion in KafkaApis.handleListClientMetricsResources, moved to java stream.

Reviewers: Mickael Maison <mickael.maison@gmail.com>, Ismael Juma <ismael@juma.me.uk>, Chia-Ping Tsai <chia7712@gmail.com>
@github-actions github-actions Bot removed the triage PRs from the community label Jan 16, 2025
pranavt84 pushed a commit to pranavt84/kafka that referenced this pull request Jan 27, 2025
Removed Optional for SharePartitionManager and ClientMetricsManager as zookeeper code is being removed. Also removed asScala and asJava conversion in KafkaApis.handleListClientMetricsResources, moved to java stream.

Reviewers: Mickael Maison <mickael.maison@gmail.com>, Ismael Juma <ismael@juma.me.uk>, Chia-Ping Tsai <chia7712@gmail.com>
airlock-confluentinc Bot pushed a commit to confluentinc/kafka that referenced this pull request Jan 27, 2025
Removed Optional for SharePartitionManager and ClientMetricsManager as zookeeper code is being removed. Also removed asScala and asJava conversion in KafkaApis.handleListClientMetricsResources, moved to java stream.

Reviewers: Mickael Maison <mickael.maison@gmail.com>, Ismael Juma <ismael@juma.me.uk>, Chia-Ping Tsai <chia7712@gmail.com>
manoj-mathivanan pushed a commit to manoj-mathivanan/kafka that referenced this pull request Feb 19, 2025
Removed Optional for SharePartitionManager and ClientMetricsManager as zookeeper code is being removed. Also removed asScala and asJava conversion in KafkaApis.handleListClientMetricsResources, moved to java stream.

Reviewers: Mickael Maison <mickael.maison@gmail.com>, Ismael Juma <ismael@juma.me.uk>, Chia-Ping Tsai <chia7712@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.

4 participants