horsteff commented on a change in pull request #7533:
URL: https://github.com/apache/pulsar/pull/7533#discussion_r522523167
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/PulsarStandaloneBuilder.java
##########
@@ -37,8 +48,8 @@ public static PulsarStandaloneBuilder instance() {
return new PulsarStandaloneBuilder();
}
- public PulsarStandaloneBuilder withConfig(ServiceConfiguration config) {
Review comment:
Normally I would agree. But in this case the method `withConfig` doesn't
do anything useful at all as the given value will be simply ignored. It is
overwritten at start of the `build` method with a freshly created
`ServiceConfiguration` instance. At first I tried to fix the build code and
keep the method, but I didn't see a useful way to do this without rewriting the
complete start process of `PulsarStandalone`. And if someone wants to change
configuration properties by code, they can do it after calling `build` and
before starting the `PulsarStandalone` instance (may be they've done it already
as it's the only way it was working). So it even was not worth the effort.
I have my doubts, that `PulsarStandaloneBuilder` is already much in use, so
backward compatibility shouldn't be a big issue. But because `withConfig` does
practically nothing, I think it's better to force those, who still use this
method (erroneously expecting the given configuration will be used), to rethink
their code by removing this method. And for new users this method would be a
source of annoyance, if they try to use it and have to discover, that it does
not work, and possibly file a new bug report which isn't easy to fix as I said
above. So I would say in this case braking backward compatibility and removing
the method is the better option.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
