Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

[improvement] Remove expensive useless String.format() in canConsumeAsync #1893

Conversation

gaoran10
Copy link
Contributor

@gaoran10 gaoran10 commented Jun 8, 2023

(cherry picked from commit fa63a4a)

This PR is used to cherry-pick the commit datastax/starlight-for-kafka@fa63a4a.

Motivation

The String.format() is an expensive operation in the method canConsumeAsync.

Modifications

Remove the String.format() operation.

Documentation

Check the box below.

Need to update docs?

  • doc-required

    (If you need help on updating docs, create a doc issue)

  • no-need-doc

    (Please explain why)

  • doc

    (If this PR contains doc changes)

@github-actions github-actions bot added the no-need-doc This pr does not need any document label Jun 8, 2023
@gaoran10 gaoran10 changed the title Remove expensive useless String.format() in canConsumeAsync [improvement] Remove expensive useless String.format() in canConsumeAsync Jun 8, 2023
@codecov
Copy link

codecov bot commented Jun 8, 2023

Codecov Report

Merging #1893 (8e9dc64) into master (d246ebb) will decrease coverage by 0.44%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1893      +/-   ##
============================================
- Coverage     18.77%   18.33%   -0.44%     
  Complexity      743      743              
============================================
  Files           184      190       +6     
  Lines         13289    13601     +312     
  Branches       1211     1260      +49     
============================================
- Hits           2495     2494       -1     
- Misses        10613    10926     +313     
  Partials        181      181              
Impacted Files Coverage Δ
...andlers/kop/security/auth/SimpleAclAuthorizer.java 0.00% <0.00%> (ø)
...native/pulsar/handlers/kop/utils/ByteBufUtils.java 0.00% <0.00%> (ø)

... and 26 files with indirect coverage changes

@@ -166,8 +166,7 @@ public CompletableFuture<Boolean> canProduceAsync(KafkaPrincipal principal, Reso

@Override
public CompletableFuture<Boolean> canConsumeAsync(KafkaPrincipal principal, Resource resource) {
checkArgument(resource.getResourceType() == ResourceType.TOPIC,
String.format("Expected resource type is TOPIC, but have [%s]", resource.getResourceType()));
checkArgument(resource.getResourceType() == ResourceType.TOPIC);
Copy link
Member

Choose a reason for hiding this comment

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

Can we extract a method like this: private void checkArgument(ResourceType expected, ResourceType actual)

Copy link
Member

Choose a reason for hiding this comment

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

And we can print the error message in the checkArgument method when the type is unequal.

Demogorgon314
Demogorgon314 previously approved these changes Jun 26, 2023
@Demogorgon314 Demogorgon314 merged commit 45bd29c into streamnative:master Jun 30, 2023
2 of 4 checks passed
BewareMyPower pushed a commit that referenced this pull request Jul 3, 2023
…sync (#1893)

(cherry picked from commit fa63a4a)

This PR is used to cherry-pick the commit
datastax/starlight-for-kafka@fa63a4a.

### Motivation

The String.format() is an expensive operation in the method
canConsumeAsync.

### Modifications

Remove the String.format() operation.

Co-authored-by: Enrico Olivelli <[email protected]>
(cherry picked from commit 45bd29c)
Demogorgon314 pushed a commit that referenced this pull request Jul 5, 2023
…sync (#1893)

(cherry picked from commit fa63a4a)

This PR is used to cherry-pick the commit
datastax/starlight-for-kafka@fa63a4a.

### Motivation

The String.format() is an expensive operation in the method
canConsumeAsync.

### Modifications

Remove the String.format() operation.

Co-authored-by: Enrico Olivelli <[email protected]>
(cherry picked from commit 45bd29c)
BewareMyPower pushed a commit that referenced this pull request Jul 17, 2023
…sync (#1893)

(cherry picked from commit fa63a4a)

This PR is used to cherry-pick the commit
datastax/starlight-for-kafka@fa63a4a.

### Motivation

The String.format() is an expensive operation in the method
canConsumeAsync.

### Modifications

Remove the String.format() operation.

Co-authored-by: Enrico Olivelli <[email protected]>
(cherry picked from commit 45bd29c)
Demogorgon314 pushed a commit to Demogorgon314/kop that referenced this pull request Aug 14, 2023
…sync (streamnative#1893)

(cherry picked from commit fa63a4a)

This PR is used to cherry-pick the commit
datastax/starlight-for-kafka@fa63a4a.

### Motivation

The String.format() is an expensive operation in the method
canConsumeAsync.

### Modifications

Remove the String.format() operation.

Co-authored-by: Enrico Olivelli <[email protected]>
(cherry picked from commit 45bd29c)

Fix conflicts by applying `checkResourceType` to `canDeleteGroupAsync`.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants