[cleanup][broker] Various cleanups #20658
Conversation
| * If the service unit is not owned, return an empty optional | ||
| */ | ||
| public Optional<URL> getWebServiceUrl(ServiceUnitId suName, LookupOptions options) throws Exception { | ||
| return getWebServiceUrlAsync(suName, options) | ||
| .get(pulsar.getConfiguration().getMetadataStoreOperationTimeoutSeconds(), SECONDS); | ||
| } | ||
|
|
||
| private CompletableFuture<Optional<URL>> internalGetWebServiceUrl(Optional<ServiceUnitId> topic, | ||
| private CompletableFuture<Optional<URL>> internalGetWebServiceUrl(ServiceUnitId topic, |
There was a problem hiding this comment.
may be we need a @Nullable annotation on ServiceUnitId here
There was a problem hiding this comment.
I like this change.
Although, I remember once I change switch case to JDK 17 match style one argues that it can introduce burden on back port code if fixes made around those code snippets. This can be a reason that @lhotari suggests we make changes firstly from the earliest maintained branch.
That's a very valid point. I'll rollback the changes on switch expressions. |
|
We also have Pulsar 2.10 which runs on JDK 8. |
| @@ -306,7 +306,7 @@ private CompletableFuture<Optional<URL>> internalGetWebServiceUrl(Optional<Servi | |||
| } | |||
| CompletableFuture<Optional<LookupResult>> future = | |||
| ExtensibleLoadManagerImpl.isLoadManagerExtensionEnabled(config) | |||
| ? loadManager.get().findBrokerServiceUrl(topic, bundle) : | |||
| ? loadManager.get().findBrokerServiceUrl(Optional.ofNullable(topic), bundle) : | |||
There was a problem hiding this comment.
Are @Nullable and Optional.ofNullable()both needed?
There was a problem hiding this comment.
I think so.
@Nullable indicates to the caller the topic can be null.
Since it can be null we need to use Optional.ofNullable() when wrapping it into an Optional.
cbornet
changed the title
Codecov Report
@@ Coverage Diff @@
## master #20658 +/- ##
============================================
+ Coverage 72.60% 73.13% +0.53%
- Complexity 32018 32102 +84
============================================
Files 1855 1871 +16
Lines 138569 138952 +383
Branches 15250 15281 +31
============================================
+ Hits 100605 101620 +1015
+ Misses 29945 29290 -655
- Partials 8019 8042 +23
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Motivation
Remove warnings in IDEA
Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: cbornet#14