Mmuzaf commented on code in PR #4263:
URL: https://github.com/apache/cassandra/pull/4263#discussion_r2219871259
##########
src/java/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommand.java:
##########
@@ -59,51 +61,107 @@ public static class GetGuardrailsConfig extends
GuardrailsConfigCommand
allowedValues = { "values", "thresholds", "flags", "others" })
private String guardrailCategory;
- @Arguments(description = "Specific names or guardrails to get
configuration of.")
+ @Option(name = { "--verbose", "-v" })
Review Comment:
Now I'm thinking that `--verbose` is probably unclear. Maybe we could choose
a better name?
`--expand-thresholds`
`--detailed`
`--show-threshold-details`
##########
src/java/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommand.java:
##########
@@ -112,54 +170,50 @@ public static class SetGuardrailsConfig extends
GuardrailsConfigCommand
{
private static final Pattern SETTER_PATTERN = Pattern.compile("^set");
- @Option(name = { "--list", "-l" },
- description = "List all available guardrails setters")
- private boolean list;
-
- @Option(name = { "--category", "-c" },
- description = "Category of guardrails to filter, can be one of
'values', 'thresholds', 'flags', 'others'.",
- allowedValues = { "values", "thresholds", "flags", "others" })
- private String guardrailCategory;
-
@Arguments(usage = "[<setter> <value1> ...]",
description = "For flags, possible values are 'true' or 'false'. " +
- "For thresholds, two values are expected, first for
warning, second for failure. " +
- "For values, one value is expected, multiple values
separated by comma.")
+ "For thresholds, two values are expected, first for
failure, second for warning. " +
+ "For values, one argument is expected, multiple values
separated by comma.")
private final List<String> args = new ArrayList<>();
@Override
public void execute(NodeProbe probe)
{
- if (!list && guardrailCategory != null)
- throw new IllegalStateException("--category/-c can be
specified only together with --list/-l");
-
- GuardrailCategory categoryEnum =
GuardrailCategory.parseCategory(guardrailCategory, probe.output().out);
-
- if (args.isEmpty() && !list)
+ if (args.isEmpty())
throw new IllegalStateException("No arguments.");
- if (list)
- display(probe, getAllSetters(probe), categoryEnum);
- else
- executeSetter(probe);
+ executeSetter(probe);
}
@Override
- public void addRow(List<InternalRow> bucket, GuardrailsMBean mBean,
Method method, String guardrailName) throws Throwable
+ public void addRow(List<InternalRow> bucket, GuardrailsMBean mBean,
List<Method> methods, String guardrailName) throws Throwable
{
- if (method.getParameterTypes().length == 1)
- constructRow(bucket, sanitizeSetterName(method),
method.getParameterTypes()[0].getName());
- else
- constructRow(bucket, sanitizeSetterName(method),
stream(method.getParameterTypes()).map(Class::getName).collect(toList()).toString());
+ if (methods.size() == 1)
+ {
+ Method method = methods.get(0);
+ if (method.getParameterTypes().length == 1)
+ constructRow(bucket, sanitizeSetterName(method),
method.getParameterTypes()[0].getName());
+ else
+ constructRow(bucket, sanitizeSetterName(method),
stream(method.getParameterTypes()).map(Class::getName).collect(toList()).toString());
+ }
}
- private List<Method> getAllSetters(NodeProbe probe)
+ private Map<String, List<Method>> getAllSetters(NodeProbe probe)
{
return
stream(probe.getGuardrailsMBean().getClass().getDeclaredMethods())
.filter(method -> method.getName().startsWith("set") &&
!method.getName().endsWith("CSV"))
.filter(method -> args.isEmpty() ||
args.contains(toSnakeCase(method.getName().substring(3))))
.sorted(comparing(Method::getName))
- .collect(toList());
+ .collect(Collectors.groupingBy(method ->
toSnakeCase(method.getName().substring(3))))
+ .entrySet()
+ .stream()
+ .sorted(Map.Entry.comparingByKey())
+ .collect(Collectors.toMap(
+ Map.Entry::getKey,
Review Comment:
There is something wrong with the indentation. I guess we lost a tab or two.
##########
src/java/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommand.java:
##########
@@ -112,54 +170,50 @@ public static class SetGuardrailsConfig extends
GuardrailsConfigCommand
{
private static final Pattern SETTER_PATTERN = Pattern.compile("^set");
- @Option(name = { "--list", "-l" },
- description = "List all available guardrails setters")
- private boolean list;
-
- @Option(name = { "--category", "-c" },
- description = "Category of guardrails to filter, can be one of
'values', 'thresholds', 'flags', 'others'.",
- allowedValues = { "values", "thresholds", "flags", "others" })
- private String guardrailCategory;
-
@Arguments(usage = "[<setter> <value1> ...]",
description = "For flags, possible values are 'true' or 'false'. " +
- "For thresholds, two values are expected, first for
warning, second for failure. " +
- "For values, one value is expected, multiple values
separated by comma.")
+ "For thresholds, two values are expected, first for
failure, second for warning. " +
+ "For values, one argument is expected, multiple values
separated by comma.")
private final List<String> args = new ArrayList<>();
@Override
public void execute(NodeProbe probe)
{
- if (!list && guardrailCategory != null)
- throw new IllegalStateException("--category/-c can be
specified only together with --list/-l");
-
- GuardrailCategory categoryEnum =
GuardrailCategory.parseCategory(guardrailCategory, probe.output().out);
-
- if (args.isEmpty() && !list)
+ if (args.isEmpty())
throw new IllegalStateException("No arguments.");
- if (list)
- display(probe, getAllSetters(probe), categoryEnum);
- else
- executeSetter(probe);
+ executeSetter(probe);
Review Comment:
The method is used only once, probably we can inline it for clarity.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
