Dump config#1160
Conversation
- add utility method to AbstractConfiguration to return config as string and log it - dump the config during the start of the BookieServer
| * @param separator | ||
| * separator to separate the configs in string | ||
| */ | ||
| public String configAsString(String separator) { |
There was a problem hiding this comment.
http configuration service is also dumping config. can we reuse (or consolidate) the logic of these two places.
There was a problem hiding this comment.
not sure what you are referring to? where is Http configuration used and how it can be reused?
There was a problem hiding this comment.
this is what I am referring to : https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/ConfigurationService.java#L52
I was thinking of adding String asJson() in AbstractConfiguration. so it can be used in 1) configuration service and 2) logging purpose as here. that is what I was talking about consolidating the logic on how we dump the configuration. so we don't need to have multiple logic on dumping configuration.
There was a problem hiding this comment.
I am ok for the json version, but it can be a second change.
I have this method which dumps the config in several projects.
I have an additional use case:
I start the bookie programmatically and then I dump the actual configuration to a file so that it is possible to use bookkeeper shell commands.
So I like this function
There was a problem hiding this comment.
well - if you are saving an configuration object to a file, you should consider using PropertiesConfiguration to save the composite configuration. commons configuration already provides extensive utils for this purpose. why do you want to reinvent this again?
I think the point here is - if we want this configuration to be dumped, it is good to have one implementation rather than multiple implementations.
There was a problem hiding this comment.
PropertiesConfiguration -> I never noticed this utility, thank you
I think having the config in JSON format inside log files is not very intuitive, but I can live with it, as far as they are "pretty printed"
There was a problem hiding this comment.
the config here is just plain key/value pairs. so there is no much difference between a properties-like format and a json format. and yes, they are pretty printed if you check this: https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/JsonUtil.java#L34
There was a problem hiding this comment.
ok..will add "String asJson()" in AbstractConfiguration class and will de-dupe the code.
- add utility method to AbstractConfiguration to return string representation of JSON string format of this config and log it - de-dupe the code
3 participants
Descriptions of the changes in this PR:
Dump config