PulsarStandaloneBuilder does not use a custom configuration set with withConfig()

[horsteff]

Describe the bug
PulsarStandaloneBuilder does not use a custom configuration set with withConfig(). It's clearly visible in the code:

    public PulsarStandalone build() {
        ServiceConfiguration config = new ServiceConfiguration();
        config.setClusterName("standalone");
        pulsarStandalone.setConfig(config);
        ...

At the beginning of PulsarStandaloneBuilder.build() a new ServiceConfiguration instance is created and set with pulsarStandalone.setConfig() overwriting any configuration instance set earlier with PulsarStandaloneBuilder.withConfig().

Additionally unlike in PulsarStandaloneStarter the PulsarStandaloneBuilder does not evaluate the configuration file for the ServiceConfiguration, so only options for LocalBookkeeperEnsemble (via the ServerConfiguration class evaluation in PulsarStandalone.start()) will be read from the configuration file, all other (e.g. managedLedgerDefaultEnsembleSize) will be ignored.

Expected behavior
PulsarStandaloneBuilder uses a custom configuration if one is set with withConfig() and should create a new ServiceConfiguration instance only if no custom configuration was set.

The configuration file is evaluated when PulsarStandalone is initialized by PulsarStandaloneBuilder.

[sijie]

@horsteff Are you interested in contributing a bug fix?

[horsteff]

@sijie Yes, that's what I had in mind. I'm already working on it and will make a pull request then.

One question: After digging a little in the code and thinking about possible fixes I would rather like to drop the withConfig method from PulsarStandaloneBuilder and replace it with a withConfigFile method. I don't see any other way (without major changes), to apply configuration settings without unexpected overwriting and in an order a user would expect. A config file is required in any case and with a withConfigFile method the PulsarStandaloneBuilder is able to read it in it's build method like the PulsarStandaloneStarter constructor does, which would fix the second part of this issue. If a user wants to change some config settings by code, it's possible by modifying the ServiceConfiguration object of the created PulsarStandalone instance after calling PulsarStandaloneBuilder.build() and before calling PulsarStandalone.start() without the need to create an own ServiceConfiguration instance. This would also make clear to the user, that modifying the config object overwrites values set via configuration file (at least those not for the LocalBookkeeperEnsemble, but that's another story).
Dropping PulsarStandaloneBuilder.withConfig shouldn't be such a problem with existing code as currently it effectively does nothing. And it seems that no other user has noticed that. 8)
Is this solution ok for you?

[sijie]

@horsteff that sounds a good solution to me.

[tisonkun]

Closed as stale. Please create a new issue if it's still relevant to the maintained versions.