accumulo
accumulo copied to clipboard
apache
Support passing a property file to the shell for setting multiple properties
Is your feature request related to a problem? Please describe. It is currently impossible to validate properties if they are set individually but are dependent on each other.
We run into this issue with the compaction service planner and various opts properties that all have to be set before validation of the planner can be performed.
https://github.com/apache/accumulo/blob/db98f7d9888cddc8b9e19aa120ea395d8d857c91/core/src/main/java/org/apache/accumulo/core/conf/Property.java#L650-L667
Describe the solution you'd like Modify the shell's config command to support setting multiple properties in a single operation from a properties file. The API already supports this via the modifyProperties method in instance, table, and namespace operations.
https://github.com/apache/accumulo/blob/0e8dc7ab8495b0e34889f4a450fdf027a5f79bc4/core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java#L129
Describe alternatives you've considered We could expand the property definitions to include more information (more json object blobs) for properties that are co-dependent. However this adds more complexity on the user to ensure the form of the property is correct.
Additional context This issue is tangentially related to #3909
This is already super trivial to do the modifyProperties command with jshell, I'm not sure it makes sense to add complexity to the shell to offer a second way of doing it. I'd rather update the docs to show an example of how to do this in jshell instead.
Modified this issue to scope down to supporting passing in a properties file to the shell for use with the multiple properties API. No custom parsing would be needed.
If we're just adding a flag to read the properties from a file, I think that's okay. I was concerned that we'd be adding a lot of complexity to parse multiple key/value pairs of properties. Reading a file is fine.
@ddanielr You marked this for 2.1. This would be a new feature. Although it's convenient, I think we're past the milestone for adding new features for 2.1, unless it's critical or is needed to address a bug. If somebody becomes dependent on this in 2.1.3, but then downgrades to 2.1.2, things will break for them. So, I think 3.1 and later makes sense for this one. What do you think?
@ddanielr You marked this for 2.1. This would be a new feature. Although it's convenient, I think we're past the milestone for adding new features for 2.1, unless it's critical or is needed to address a bug. If somebody becomes dependent on this in 2.1.3, but then downgrades to 2.1.2, things will break for them. So, I think 3.1 and later makes sense for this one. What do you think?
That sounds fine! I agree that if they become dependent on it and have to downgrade It would be a major inconvenience.