You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/07/09 15:39:56 UTC

[GitHub] [pulsar] horsteff opened a new issue #7492: PulsarStandaloneBuilder does not use a custom configuration set with withConfig()

horsteff opened a new issue #7492:
URL: https://github.com/apache/pulsar/issues/7492


   **Describe the bug**
   `PulsarStandaloneBuilder` does not use a custom configuration set with `withConfig()`. It's clearly visible in the code:
   ```java
       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`.
   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [pulsar] sijie commented on issue #7492: PulsarStandaloneBuilder does not use a custom configuration set with withConfig()

Posted by GitBox <gi...@apache.org>.

sijie commented on issue #7492:
URL: https://github.com/apache/pulsar/issues/7492#issuecomment-656438071


   @horsteff Are you interested in contributing a bug fix?


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [pulsar] sijie commented on issue #7492: PulsarStandaloneBuilder does not use a custom configuration set with withConfig()

Posted by GitBox <gi...@apache.org>.

sijie commented on issue #7492:
URL: https://github.com/apache/pulsar/issues/7492#issuecomment-657922721


   @horsteff that sounds a good solution to me.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [pulsar] horsteff commented on issue #7492: PulsarStandaloneBuilder does not use a custom configuration set with withConfig()

Posted by GitBox <gi...@apache.org>.

horsteff commented on issue #7492:
URL: https://github.com/apache/pulsar/issues/7492#issuecomment-656769281

@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?

----------------------------------------------------------------
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:
users@infra.apache.org