You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by "anmolnar (via GitHub)" <gi...@apache.org> on 2025/10/29 16:15:33 UTC
[PR] ZOOKEEPER-4988: Add [-R] recursive option to getAcl cli command [zookeeper]
anmolnar opened a new pull request, #2334:
URL: https://github.com/apache/zookeeper/pull/2334
(no comment)
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ZOOKEEPER-4988: Add [-R] recursive option to getAcl cli command [zookeeper]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #2334:
URL: https://github.com/apache/zookeeper/pull/2334#discussion_r2566708974
##########
zookeeper-server/src/main/java/org/apache/zookeeper/cli/GetAclCommand.java:
##########
@@ -38,10 +38,11 @@ public class GetAclCommand extends CliCommand {
static {
options.addOption("s", false, "stats");
+ options.addOption("R", false, "recursive");
Review Comment:
Why upper-case? Was lower-case already used? I guess it doesn't matter, but I'm curious.
##########
zookeeper-server/src/main/java/org/apache/zookeeper/cli/GetAclCommand.java:
##########
@@ -38,10 +38,11 @@ public class GetAclCommand extends CliCommand {
static {
options.addOption("s", false, "stats");
+ options.addOption("R", false, "recursive");
}
public GetAclCommand() {
- super("getAcl", "[-s] path");
+ super("getAcl", "[-s] [-R] path");
Review Comment:
I think commons-cli, which it looks like ZK is using, can generate help documentation automatically. It shouldn't be necessary to maintain a description that is passed around. But, that's out of scope of this PR. I was just surprised to see a help document being manually updated in this PR.
##########
zookeeper-server/src/main/java/org/apache/zookeeper/cli/GetAclCommand.java:
##########
@@ -65,21 +66,37 @@ public boolean exec() throws CliException {
String path = args[1];
Stat stat = new Stat();
List<ACL> acl;
+ boolean recursive = cl.hasOption("R");
try {
- acl = zk.getACL(path, stat);
+ if (recursive) {
+ ZKUtil.visitSubTreeDFS(zk, path, false, (rc, path1, ctx, name) -> {
+ try {
+ out.println(path1);
+ printAcl(zk.getACL(path1, stat), stat);
Review Comment:
Rather than print on separate lines, it would be better if these were formatted such that they could be on one line, so the output can be more easily parsed by somebody redirecting this elsewhere. Could the path be appended with something like ` # /path/to/node`?
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ZOOKEEPER-4988: Add [-R] recursive option to getAcl cli command [zookeeper]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #2334:
URL: https://github.com/apache/zookeeper/pull/2334#discussion_r2566799664
##########
zookeeper-server/src/main/java/org/apache/zookeeper/cli/GetAclCommand.java:
##########
@@ -38,10 +38,11 @@ public class GetAclCommand extends CliCommand {
static {
options.addOption("s", false, "stats");
+ options.addOption("R", false, "recursive");
Review Comment:
To be consistent with [SetAclCommand](https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SetAclCommand.java#L50)
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ZOOKEEPER-4988: Add [-R] recursive option to getAcl cli command [zookeeper]
Posted by "anmolnar (via GitHub)" <gi...@apache.org>.
anmolnar commented on code in PR #2334:
URL: https://github.com/apache/zookeeper/pull/2334#discussion_r2566796258
##########
zookeeper-server/src/main/java/org/apache/zookeeper/cli/GetAclCommand.java:
##########
@@ -65,21 +66,37 @@ public boolean exec() throws CliException {
String path = args[1];
Stat stat = new Stat();
List<ACL> acl;
+ boolean recursive = cl.hasOption("R");
try {
- acl = zk.getACL(path, stat);
+ if (recursive) {
+ ZKUtil.visitSubTreeDFS(zk, path, false, (rc, path1, ctx, name) -> {
+ try {
+ out.println(path1);
+ printAcl(zk.getACL(path1, stat), stat);
Review Comment:
Makes sense to me, let me take a look.
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org