keith-turner commented on code in PR #6012:
URL: https://github.com/apache/accumulo/pull/6012#discussion_r2607636674
##########
shell/src/main/java/org/apache/accumulo/shell/commands/CreateTableCommand.java:
##########
@@ -152,17 +168,29 @@ public int execute(final String fullCommand, final
CommandLine cl, final Shell s
}
// Copy configuration options if flag was set
- Map<String,String> srcTableConfig = null;
- if (cl.hasOption(createTableOptCopyConfig.getOpt())) {
- String srcTable = cl.getOptionValue(createTableOptCopyConfig.getOpt());
- if (cl.hasOption(createTableOptExcludeParentProps.getLongOpt())) {
+ Map<String,String> srcTableConfig;
+ var hasCopyConfigOpt = cl.hasOption(createTableOptCopyConfig);
+ var hasCopyPropsOpt = cl.hasOption(createTableOptCopyProps);
+ if (hasCopyConfigOpt || hasCopyPropsOpt) {
+ if (hasCopyConfigOpt) {
+ String srcTable = cl.getOptionValue(createTableOptCopyConfig.getOpt());
+ if (cl.hasOption(createTableOptExcludeParentProps.getLongOpt())) {
+ Shell.log.warn(
+ "{} is deprecated and will be removed in a future version. Use
{} instead.",
+ createTableOptExcludeParentProps.getLongOpt(),
createTableOptCopyProps.getLongOpt());
+ // copy properties, excludes parent properties in configuration
+ srcTableConfig =
+
shellState.getAccumuloClient().tableOperations().getTableProperties(srcTable);
+ } else {
+ // copy configuration (include parent properties)
+ srcTableConfig =
Review Comment:
IMO the copy config option was only useful when used w/ the exclude parent
props option. Since exclude parent is being deprecated, maybe we should also
deprecate copy config also.
The behavior of the copy config option by itself is very confusing, it
copies default,site (from some random server), system, and namespace config
into the table. Then the way the shell config command works it does not show
this happened. So the command silently locks in config from higher levels.
Below is an experiment I just ran through to confirm this.
1. Add `table.file.max=20` to accumulo.properties file
2. Restart cluster
3. Create table w/ copy config and it copied `table.file.max` into the new
tables config from the site file. However when printing table config in the
shell it does not show this (because the table and site have the same value, so
it only shows site).
4. Remove `table.file.max=20` from accumulo.properties file and restart
cluster
5. Print tables config in the shell, now it shows that the table has
`table.file.max=20` which came from the site file.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
