-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Automatically generate datanode.conf.example #19141
base: master
Are you sure you want to change the base?
Conversation
…onfiguration options
.map(f -> toConfigurationEntry(f, configurationBean)); | ||
} | ||
|
||
private static List<Object> getDatanodeConfigurationBeans() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should already move that out of the generator to be able to move the generator to a server package more easily?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any simple idea how/where to get this?
|
||
public class ConfigFileDocsPrinter implements DocsPrinter { | ||
|
||
public static final String DATANODE_CONFIG_HEADER = """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here for the header. Maybe move it out of the printer, and possibly read it from a resource to avoid the huge string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is only a tiny difference between graylog.conf and datanode.conf header. Maybe it would be enough to provide string headline and use the whole header as a template?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good.
@bernd what do you think? Is this a valid and usable approach? Can we merge it? Thanks! |
@todvora, I like the general approach! 👍 We should improve a few details before we ship the generated config, though.
|
Easy to implement, done.
That happens already. Properties that are mandatory and don't have default value, these that users need to fill in, are ordered at the top of the configuration file. Otherwise the order follows order of the properties in the java config beans. Reordering in java leads to reodering in the config file. Seems natural way to handle this.
Fixed
Indeed. IDK how to add this in the current situation without significant overhead in the java config files. From my POV, the sections should correspond to java configuration classes: one class == one section. Then we can add a heading and group description to the class itself. But for now, at least in the datanode, we have (almost)everything cramped into one configuration file. Given this configuration part:
I see a JwtTokenConfiguration.java class with its own header This would be rather simple to implement but would require significant changes in the configuration classes and all of their usages. Is that something we'd consider? I personally think it would offer benefits for the code base as well. Now we drag the whole configuration everywhere, or even worse from the maintenance perspective, use named injects, which make any refactoring a lot harder and error prone. My suggestion - for now go without sections. Split datanode configuration to more specific config beans. Add section documentation to the beans and see if that will work well for us. |
Updated the generated configuration in the PR description. |
👍 Thanks!
Ah cool. Sorry, I have missed that. I think my eye caught the
👍 Thanks!
I think we could add an optional "section" value to the |
Generate the datanode.conf.example file with all possible configuration options. Also generate the csv documentation for the same.
/nocl
Description
This PR adds two maven tasks that generate datanode.conf.example and datanode-conf-docs.csv files. The conf.example will be included in our dist packages. The csv may be used for documentation purposes.
The datanode assembly now includes this generated file instead of the manually created.
Motivation and Context
Automate and enforce documentation of configuration options.
How Has This Been Tested?
Manually
Screenshots (if appropriate):
Types of changes
Checklist: