Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
jerryshao merged PR #5714: URL: https://github.com/apache/gravitino/pull/5714 -- 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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1864725205
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/Command.java:
##
@@ -69,4 +79,19 @@ protected GravitinoAdminClient buildAdminClient() {
return GravitinoAdminClient.builder(url).build();
}
}
+
+ /**
+ * Outputs the entity to the console.
+ *
+ * @param entity The entity to output.
+ */
+ protected void output(T entity) {
+if (outputFormat != null) {
+ if (outputFormat.equals(OUTPUT_FORMAT_TABLE)) {
+TableFormat.output(entity);
+ } else if (outputFormat.equals(OUTPUT_FORMAT_PLAIN)) {
+PlainFormat.output(entity);
+ }
+}
Review Comment:
Fixed in the commit:
https://github.com/apache/gravitino/pull/5714/commits/b6b9b520cbd2e69be56d5e5f33fda2cb51a2c7f6
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin commented on PR #5606: URL: https://github.com/apache/gravitino/pull/5606#issuecomment-2507253470 Reopen the PR -- 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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin commented on PR #5606: URL: https://github.com/apache/gravitino/pull/5606#issuecomment-2507252963 > Hi @xunliu, I pushed a commit and Github shows "Processing updates" in the past 3 hours. Do you know how to fix this problem?  I tried closing and reopening the PR to see if GitHub could back to normal. However, after closing the PR, it seems I can no longer reopen it. Ref: https://github.com/orgs/community/discussions/51962 -- 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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin commented on PR #5606: URL: https://github.com/apache/gravitino/pull/5606#issuecomment-2507250940 I tried closing and reopening the PR to see if GitHub could back to normal. Ref: https://github.com/orgs/community/discussions/51962 -- 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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin commented on PR #5606: URL: https://github.com/apache/gravitino/pull/5606#issuecomment-2507250326 > Hi @xunliu, I pushed a commit and Github shows "Processing updates" in the past 3 hours. Do you know how to fix this problem?  I tried closing and reopening the PR to see if GitHub could back to normal. However, after closing the PR, it seems I can no longer reopen it. Ref: https://github.com/orgs/community/discussions/51962 -- 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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin closed pull request #5606: [#5506] feat(CLI): Table formatted output URL: https://github.com/apache/gravitino/pull/5606 -- 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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin commented on PR #5606: URL: https://github.com/apache/gravitino/pull/5606#issuecomment-2507176587 Hi @xunliu, I pushed a commit and Github shows "Processing updates" in the past 3 hours. Do you know how to fix this problem?  -- 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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
justinmclean commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1862711817
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/Command.java:
##
@@ -69,4 +79,19 @@ protected GravitinoAdminClient buildAdminClient() {
return GravitinoAdminClient.builder(url).build();
}
}
+
+ /**
+ * Outputs the entity to the console.
+ *
+ * @param entity The entity to output.
+ */
+ protected void output(T entity) {
+if (outputFormat != null) {
+ if (outputFormat.equals(OUTPUT_FORMAT_TABLE)) {
+TableFormat.output(entity);
+ } else if (outputFormat.equals(OUTPUT_FORMAT_PLAIN)) {
+PlainFormat.output(entity);
+ }
+}
Review Comment:
It should default to the plain output rather than output nothing. Also
`--output plain` should be an option. `--output ` should give
an unsupported format message.
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1862258098
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/Command.java:
##
@@ -69,4 +79,19 @@ protected GravitinoAdminClient buildAdminClient() {
return GravitinoAdminClient.builder(url).build();
}
}
+
+ /**
+ * Outputs the entity to the console.
+ *
+ * @param entity The entity to output.
+ */
+ protected void output(T entity) {
+if (outputFormat != null) {
+ if (outputFormat.equals(OUTPUT_FORMAT_TABLE)) {
+TableFormat.output(entity);
+ } else if (outputFormat.equals(OUTPUT_FORMAT_PLAIN)) {
+PlainFormat.output(entity);
+ }
+}
Review Comment:
I think @justinmclean may have a different opinion. Could you please explain
about it? https://github.com/apache/gravitino/pull/5606#discussion_r1859275498
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1862258098
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/Command.java:
##
@@ -69,4 +79,19 @@ protected GravitinoAdminClient buildAdminClient() {
return GravitinoAdminClient.builder(url).build();
}
}
+
+ /**
+ * Outputs the entity to the console.
+ *
+ * @param entity The entity to output.
+ */
+ protected void output(T entity) {
+if (outputFormat != null) {
+ if (outputFormat.equals(OUTPUT_FORMAT_TABLE)) {
+TableFormat.output(entity);
+ } else if (outputFormat.equals(OUTPUT_FORMAT_PLAIN)) {
+PlainFormat.output(entity);
+ }
+}
Review Comment:
@justinmclean may have a different opinion. Could you please explain about
it? https://github.com/apache/gravitino/pull/5606#discussion_r1859275498
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
xunliu commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1862245855
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/Command.java:
##
@@ -69,4 +79,19 @@ protected GravitinoAdminClient buildAdminClient() {
return GravitinoAdminClient.builder(url).build();
}
}
+
+ /**
+ * Outputs the entity to the console.
+ *
+ * @param entity The entity to output.
+ */
+ protected void output(T entity) {
+if (outputFormat != null) {
+ if (outputFormat.equals(OUTPUT_FORMAT_TABLE)) {
+TableFormat.output(entity);
+ } else if (outputFormat.equals(OUTPUT_FORMAT_PLAIN)) {
+PlainFormat.output(entity);
+ }
+}
Review Comment:
@waukin I think this has a problem.
If no output format is specified, will nothing be output?
We should default use Plain format to output.
Please check it.
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
tengqm commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1862223611
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/Command.java:
##
@@ -69,4 +79,19 @@ protected GravitinoAdminClient buildAdminClient() {
return GravitinoAdminClient.builder(url).build();
}
}
+
+ /**
+ * Outputs the entity to the console.
+ *
+ * @param entity The entity to output.
+ */
+ protected void output(T entity) {
+if (outputFormat != null) {
+ if (outputFormat.equals(OUTPUT_FORMAT_TABLE)) {
+TableFormat.output(entity);
+ } else if (outputFormat.equals(OUTPUT_FORMAT_PLAIN)) {
+PlainFormat.output(entity);
+ }
+}
Review Comment:
I think this logic can be improved/simplified.
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1862117539
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/CatalogDetails.java:
##
@@ -37,9 +37,11 @@ public class CatalogDetails extends Command {
* @param ignoreVersions If true don't check the client/server versions
match.
* @param metalake The name of the metalake.
* @param catalog The name of the catalog.
+ * @param outputFormat The output format.
*/
- public CatalogDetails(String url, boolean ignoreVersions, String metalake,
String catalog) {
-super(url, ignoreVersions);
+ public CatalogDetails(
Review Comment:
Added in this commit:
https://github.com/apache/gravitino/pull/5606/commits/5ac9e629d0be8247082d7285ee62fc517cdb018b
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
xunliu commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1861465557
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/CatalogDetails.java:
##
@@ -37,9 +37,11 @@ public class CatalogDetails extends Command {
* @param ignoreVersions If true don't check the client/server versions
match.
* @param metalake The name of the metalake.
* @param catalog The name of the catalog.
+ * @param outputFormat The output format.
*/
- public CatalogDetails(String url, boolean ignoreVersions, String metalake,
String catalog) {
-super(url, ignoreVersions);
+ public CatalogDetails(
Review Comment:
hi @waukin Please add Catalog output ITs in the `TableFormatOutputIT.java`
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1860629621
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/Command.java:
##
@@ -69,4 +77,17 @@ protected GravitinoAdminClient buildAdminClient() {
return GravitinoAdminClient.builder(url).build();
}
}
+
+ /**
+ * Outputs the entity to the console.
+ *
+ * @param entity The entity to output.
+ */
+ protected void output(T entity) {
+if (outputFormat != null && outputFormat.equals("table")) {
+ TableFormat.output(entity);
+} else {
Review Comment:
Understand. This has been fixed in commit:
https://github.com/apache/gravitino/pull/5606/commits/21dabc397c4299ace464fe28148c8a97fdc807e8
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1860631880
##
clients/cli/src/test/java/org/apache/gravitino/cli/integration/test/TableFormatOutputIT.java:
##
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.cli.integration.test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
+import org.apache.gravitino.cli.GravitinoOptions;
+import org.apache.gravitino.cli.Main;
+import org.apache.gravitino.integration.test.util.BaseIT;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+public class TableFormatOutputIT extends BaseIT {
+ private String gravitinoUrl;
+
+ @BeforeAll
+ public void startUp() {
+gravitinoUrl = String.format("http://127.0.0.1:%d";,
getGravitinoServerPort());
+ }
+
+ @Test
+ public void testMetalakeListCommand() {
Review Comment:
`testMetalakeDetailsCommand()` has been added in commit:
https://github.com/apache/gravitino/pull/5606/commits/fa5bc716abf938345c023ccf1bbf2dea13d86716
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
xunliu commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1859652820
##
clients/cli/src/test/java/org/apache/gravitino/cli/integration/test/TableFormatOutputIT.java:
##
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.cli.integration.test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
+import org.apache.gravitino.cli.GravitinoOptions;
+import org.apache.gravitino.cli.Main;
+import org.apache.gravitino.integration.test.util.BaseIT;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+public class TableFormatOutputIT extends BaseIT {
+ private String gravitinoUrl;
+
+ @BeforeAll
+ public void startUp() {
+gravitinoUrl = String.format("http://127.0.0.1:%d";,
getGravitinoServerPort());
+ }
+
+ @Test
+ public void testMetalakeListCommand() {
Review Comment:
hi @waukin
Please add `testMetalakeDetailsCommand()`, Thanks.
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
justinmclean commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1859275498
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/Command.java:
##
@@ -69,4 +77,17 @@ protected GravitinoAdminClient buildAdminClient() {
return GravitinoAdminClient.builder(url).build();
}
}
+
+ /**
+ * Outputs the entity to the console.
+ *
+ * @param entity The entity to output.
+ */
+ protected void output(T entity) {
+if (outputFormat != null && outputFormat.equals("table")) {
+ TableFormat.output(entity);
+} else {
Review Comment:
Sure, but imagine a user is scripting this, having a "--output $format"
(where $format could be "plain" or "table") would be helpful, and also, more
formats (e.g. @JSON) might be added in the future. Putting that check in would
mean that the script would be more future proof.
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
justinmclean commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1859275498
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/Command.java:
##
@@ -69,4 +77,17 @@ protected GravitinoAdminClient buildAdminClient() {
return GravitinoAdminClient.builder(url).build();
}
}
+
+ /**
+ * Outputs the entity to the console.
+ *
+ * @param entity The entity to output.
+ */
+ protected void output(T entity) {
+if (outputFormat != null && outputFormat.equals("table")) {
+ TableFormat.output(entity);
+} else {
Review Comment:
Sure, but imagine a user is scripting this, having a "--output $format"
(where $format could be "plain" or "table") would be helpful, and also, more
formats (e.g. "JSON") might be added in the future. Putting that check in
would mean that the script would be more future-proof.
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1858708524
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/Command.java:
##
@@ -69,4 +77,17 @@ protected GravitinoAdminClient buildAdminClient() {
return GravitinoAdminClient.builder(url).build();
}
}
+
+ /**
+ * Outputs the entity to the console.
+ *
+ * @param entity The entity to output.
+ */
+ protected void output(T entity) {
+if (outputFormat != null && outputFormat.equals("table")) {
+ TableFormat.output(entity);
+} else {
Review Comment:
If the user does not specify the output , the default is PlainFormat. Static
variables are used to store "table" and "plain"; this has been fixed in commit:
https://github.com/apache/gravitino/pull/5606/commits/675bf45496db13a65dd1d5238b4dd20eb44f08de
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
justinmclean commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1857739848
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/Command.java:
##
@@ -69,4 +77,17 @@ protected GravitinoAdminClient buildAdminClient() {
return GravitinoAdminClient.builder(url).build();
}
}
+
+ /**
+ * Outputs the entity to the console.
+ *
+ * @param entity The entity to output.
+ */
+ protected void output(T entity) {
+if (outputFormat != null && outputFormat.equals("table")) {
+ TableFormat.output(entity);
+} else {
Review Comment:
Should also check if the output is "plain"? May not be best to hardcode
"table"?
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
xunliu commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1856499732
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/MetalakeDetails.java:
##
@@ -33,27 +34,24 @@ public class MetalakeDetails extends Command {
* @param url The URL of the Gravitino server.
* @param ignoreVersions If true don't check the client/server versions
match.
* @param metalake The name of the metalake.
+ * @param OutputFormat The output format.
*/
- public MetalakeDetails(String url, boolean ignoreVersions, String metalake) {
-super(url, ignoreVersions);
+ public MetalakeDetails(String url, boolean ignoreVersions, String metalake,
String OutputFormat) {
+super(url, ignoreVersions, OutputFormat);
Review Comment:
DONE
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/MetalakeDetails.java:
##
@@ -33,27 +34,24 @@ public class MetalakeDetails extends Command {
* @param url The URL of the Gravitino server.
* @param ignoreVersions If true don't check the client/server versions
match.
* @param metalake The name of the metalake.
+ * @param OutputFormat The output format.
Review Comment:
DONE
##
clients/cli/src/test/java/org/apache/gravitino/cli/TestMetalakeCommands.java:
##
@@ -59,7 +59,9 @@ void testListMetalakesCommand() {
spy(
new GravitinoCommandLine(
mockCommandLine, mockOptions, CommandEntities.METALAKE,
CommandActions.LIST));
-
doReturn(mockList).when(commandLine).newListMetalakes(GravitinoCommandLine.DEFAULT_URL,
false);
+doReturn(mockList)
+.when(commandLine)
+.newListMetalakes(GravitinoCommandLine.DEFAULT_URL, false, "");
Review Comment:
DONE
##
clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java:
##
@@ -89,6 +89,7 @@ public Options options() {
// Force delete entity and rename metalake operations
options.addOption(createSimpleOption("f", FORCE, "force operation"));
+options.addOption(createArgOption(null, OUTPUT, "output format (table)"));
Review Comment:
DONE
##
clients/cli/src/test/java/org/apache/gravitino/cli/TestMetalakeCommands.java:
##
@@ -76,7 +78,7 @@ void testMetalakeDetailsCommand() {
mockCommandLine, mockOptions, CommandEntities.METALAKE,
CommandActions.DETAILS));
doReturn(mockDetails)
.when(commandLine)
-.newMetalakeDetails(GravitinoCommandLine.DEFAULT_URL, false,
"metalake_demo");
+.newMetalakeDetails(GravitinoCommandLine.DEFAULT_URL, false,
"metalake_demo", "");
Review Comment:
DONE
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
xunliu commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1856497861
##
clients/cli/src/main/java/org/apache/gravitino/cli/outputs/TableFormats.java:
##
@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.cli.outputs;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import org.apache.gravitino.Metalake;
+
+/** TableFormats to print a pretty table to standard out. */
+public class TableFormats {
Review Comment:
DONE
##
clients/cli/src/main/java/org/apache/gravitino/cli/outputs/CommaSeparatorFormats.java:
##
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.cli.outputs;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.gravitino.Metalake;
+
+/** Comma separator format to print a pretty string to standard out. */
+public class CommaSeparatorFormats {
Review Comment:
DONE
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
xunliu commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1856497861
##
clients/cli/src/main/java/org/apache/gravitino/cli/outputs/TableFormats.java:
##
@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.cli.outputs;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import org.apache.gravitino.Metalake;
+
+/** TableFormats to print a pretty table to standard out. */
+public class TableFormats {
Review Comment:
DONE
##
clients/cli/src/main/java/org/apache/gravitino/cli/outputs/TableFormats.java:
##
@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.cli.outputs;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import org.apache.gravitino.Metalake;
+
+/** TableFormats to print a pretty table to standard out. */
+public class TableFormats {
Review Comment:
DONE
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
justinmclean commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1855909668
##
clients/cli/src/main/java/org/apache/gravitino/cli/outputs/TableFormats.java:
##
@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.cli.outputs;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import org.apache.gravitino.Metalake;
+
+/** TableFormats to print a pretty table to standard out. */
+public class TableFormats {
Review Comment:
I think this class should be called TableFormat or TableFormatted.
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
justinmclean commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1855905530
##
clients/cli/src/main/java/org/apache/gravitino/cli/outputs/CommaSeparatorFormats.java:
##
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.cli.outputs;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.gravitino.Metalake;
+
+/** Comma separator format to print a pretty string to standard out. */
+public class CommaSeparatorFormats {
Review Comment:
The class should be called CommaSeperatedFormat or CommaSeperatedFormats as
the term is comma separated format or comma separated values (CSV).
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
justinmclean commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1855892542
##
clients/cli/src/test/java/org/apache/gravitino/cli/TestMetalakeCommands.java:
##
@@ -59,7 +59,9 @@ void testListMetalakesCommand() {
spy(
new GravitinoCommandLine(
mockCommandLine, mockOptions, CommandEntities.METALAKE,
CommandActions.LIST));
-
doReturn(mockList).when(commandLine).newListMetalakes(GravitinoCommandLine.DEFAULT_URL,
false);
+doReturn(mockList)
+.when(commandLine)
+.newListMetalakes(GravitinoCommandLine.DEFAULT_URL, false, "");
Review Comment:
If --output is not specified on teh CLI then its value should be null not an
empty string?
##
clients/cli/src/test/java/org/apache/gravitino/cli/TestMetalakeCommands.java:
##
@@ -76,7 +78,7 @@ void testMetalakeDetailsCommand() {
mockCommandLine, mockOptions, CommandEntities.METALAKE,
CommandActions.DETAILS));
doReturn(mockDetails)
.when(commandLine)
-.newMetalakeDetails(GravitinoCommandLine.DEFAULT_URL, false,
"metalake_demo");
+.newMetalakeDetails(GravitinoCommandLine.DEFAULT_URL, false,
"metalake_demo", "");
Review Comment:
If --output is not specified on teh CLI then its value should be null not an
empty string?
##
clients/cli/src/test/java/org/apache/gravitino/cli/TestMetalakeCommands.java:
##
@@ -76,7 +78,7 @@ void testMetalakeDetailsCommand() {
mockCommandLine, mockOptions, CommandEntities.METALAKE,
CommandActions.DETAILS));
doReturn(mockDetails)
.when(commandLine)
-.newMetalakeDetails(GravitinoCommandLine.DEFAULT_URL, false,
"metalake_demo");
+.newMetalakeDetails(GravitinoCommandLine.DEFAULT_URL, false,
"metalake_demo", "");
Review Comment:
If --output is not specified on the CLI then its value should be null not an
empty string?
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
justinmclean commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1855895395
##
clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java:
##
@@ -89,6 +89,7 @@ public Options options() {
// Force delete entity and rename metalake operations
options.addOption(createSimpleOption("f", FORCE, "force operation"));
+options.addOption(createArgOption(null, OUTPUT, "output format (table)"));
Review Comment:
Perhaps also name the default format e.g. "plain"?
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
justinmclean commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1855892542
##
clients/cli/src/test/java/org/apache/gravitino/cli/TestMetalakeCommands.java:
##
@@ -59,7 +59,9 @@ void testListMetalakesCommand() {
spy(
new GravitinoCommandLine(
mockCommandLine, mockOptions, CommandEntities.METALAKE,
CommandActions.LIST));
-
doReturn(mockList).when(commandLine).newListMetalakes(GravitinoCommandLine.DEFAULT_URL,
false);
+doReturn(mockList)
+.when(commandLine)
+.newListMetalakes(GravitinoCommandLine.DEFAULT_URL, false, "");
Review Comment:
If --output is not specified on the CLI then its value should be null not an
empty string?
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
justinmclean commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1855889032
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/MetalakeDetails.java:
##
@@ -33,27 +34,24 @@ public class MetalakeDetails extends Command {
* @param url The URL of the Gravitino server.
* @param ignoreVersions If true don't check the client/server versions
match.
* @param metalake The name of the metalake.
+ * @param OutputFormat The output format.
*/
- public MetalakeDetails(String url, boolean ignoreVersions, String metalake) {
-super(url, ignoreVersions);
+ public MetalakeDetails(String url, boolean ignoreVersions, String metalake,
String OutputFormat) {
+super(url, ignoreVersions, OutputFormat);
Review Comment:
Same here
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
justinmclean commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1855887054
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/MetalakeDetails.java:
##
@@ -33,27 +34,24 @@ public class MetalakeDetails extends Command {
* @param url The URL of the Gravitino server.
* @param ignoreVersions If true don't check the client/server versions
match.
* @param metalake The name of the metalake.
+ * @param OutputFormat The output format.
Review Comment:
I think this should be outputFormat
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
xunliu commented on PR #5606: URL: https://github.com/apache/gravitino/pull/5606#issuecomment-2496580672 hi @waukin I will help you improve your this PR, Please wait. -- 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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
tengqm commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1853165264
##
clients/cli/src/main/java/org/apache/gravitino/cli/CommandLineOutputs.java:
##
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.cli;
+
+import org.apache.commons.cli.Option;
+import org.apache.gravitino.cli.outputs.StringFormat;
+import org.apache.gravitino.cli.outputs.TableFormat;
+
+/** Outputs different format for the CLI results. */
+public class CommandLineOutputs {
+ public static void output(T entity) {
+if (GravitinoCommandLine.getOptions().hasOption("output")) {
Review Comment:
A test like this is unnecessary.
The `getOption()` method returns null if there is no such an option.
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
justinmclean commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1852970659
##
clients/cli/src/main/java/org/apache/gravitino/cli/CommandLineOutputs.java:
##
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.cli;
+
+import org.apache.commons.cli.Option;
+import org.apache.gravitino.cli.outputs.StringFormat;
+import org.apache.gravitino.cli.outputs.TableFormat;
+
+/** Outputs different format for the CLI results. */
+public class CommandLineOutputs {
+ public static void output(T entity) {
+if (GravitinoCommandLine.getOptions().hasOption("output")) {
+ Option output = GravitinoCommandLine.getOptions().getOption("output");
+ if (output.getValue().equals("table")) {
Review Comment:
I would suggest passing the value of `line.getOptionValue("table")` to the
method. I would also not hard code "table".
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1852220285
##
clients/cli/src/main/java/org/apache/gravitino/cli/CommandLineOutputs.java:
##
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.cli;
+
+import org.apache.commons.cli.Option;
+import org.apache.gravitino.cli.outputs.StringFormat;
+import org.apache.gravitino.cli.outputs.TableFormat;
+
+/** Outputs different format for the CLI results. */
+public class CommandLineOutputs {
+ public static void output(T entity) {
+if (GravitinoCommandLine.getOptions().hasOption("output")) {
+ Option output = GravitinoCommandLine.getOptions().getOption("output");
+ if (output.getValue().equals("table")) {
Review Comment:
Hi @xunliu, I found that `output.getValue()` always returns `null`. To
correctly parse and retrieve the value of the `output` option, we might need
the `CommandLine` object here. Do you have any suggestions?
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1852220285
##
clients/cli/src/main/java/org/apache/gravitino/cli/CommandLineOutputs.java:
##
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.cli;
+
+import org.apache.commons.cli.Option;
+import org.apache.gravitino.cli.outputs.StringFormat;
+import org.apache.gravitino.cli.outputs.TableFormat;
+
+/** Outputs different format for the CLI results. */
+public class CommandLineOutputs {
+ public static void output(T entity) {
+if (GravitinoCommandLine.getOptions().hasOption("output")) {
+ Option output = GravitinoCommandLine.getOptions().getOption("output");
+ if (output.getValue().equals("table")) {
Review Comment:
Hi @xunliu, I found that `output.getValue()` always returns `null`. To
correctly parse and retrieve the value of the `output` option, we might need
the `CommandLine` object here? Do you have any suggestions?
I test with below command.
```shell
gcli metalake -m --output table details
```
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1852220285
##
clients/cli/src/main/java/org/apache/gravitino/cli/CommandLineOutputs.java:
##
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.cli;
+
+import org.apache.commons.cli.Option;
+import org.apache.gravitino.cli.outputs.StringFormat;
+import org.apache.gravitino.cli.outputs.TableFormat;
+
+/** Outputs different format for the CLI results. */
+public class CommandLineOutputs {
+ public static void output(T entity) {
+if (GravitinoCommandLine.getOptions().hasOption("output")) {
+ Option output = GravitinoCommandLine.getOptions().getOption("output");
+ if (output.getValue().equals("table")) {
Review Comment:
Hi @xunliu, I found that `output.getValue()` always returns `null`. To
correctly parse and retrieve the value of the `output` option, we might need
the `CommandLine` object here. Do you have any suggestions?
I test with below command.
```shell
gcli metalake -m --output table details
```
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1852220285
##
clients/cli/src/main/java/org/apache/gravitino/cli/CommandLineOutputs.java:
##
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.cli;
+
+import org.apache.commons.cli.Option;
+import org.apache.gravitino.cli.outputs.StringFormat;
+import org.apache.gravitino.cli.outputs.TableFormat;
+
+/** Outputs different format for the CLI results. */
+public class CommandLineOutputs {
+ public static void output(T entity) {
+if (GravitinoCommandLine.getOptions().hasOption("output")) {
+ Option output = GravitinoCommandLine.getOptions().getOption("output");
+ if (output.getValue().equals("table")) {
Review Comment:
Hi @xunliu, I found that `output.getValue()` always returns `null`. To
correctly parse and retrieve the value of the `output` option, we might need
the `CommandLine` object here. Do you have any suggestions?
I test with below command.
```shell
gcli metalake -m test --output table details
```
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
xunliu commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1847846692
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/ListMetalakes.java:
##
@@ -50,13 +51,12 @@ public void handle() {
return;
}
-List metalakeNames = new ArrayList<>();
+List headers = Arrays.asList("metalake");
+List> rows = new ArrayList<>();
for (int i = 0; i < metalakes.length; i++) {
- metalakeNames.add(metalakes[i].name());
+ rows.add(Arrays.asList(metalakes[i].name()));
}
-
-String all = Joiner.on(System.lineSeparator()).join(metalakeNames);
-
-System.out.println(all.toString());
+TablePrinter tablePrinter = new TablePrinter();
+tablePrinter.print(headers, rows);
Review Comment:
hi @justinmclean
I think we only need one format output, why do we needs kept two different
output?
Can you explain?
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
xunliu commented on PR #5606: URL: https://github.com/apache/gravitino/pull/5606#issuecomment-2485699094 hi @waukin I help you improve this PR, please get the latest code in this PR to continue development, Tanks! -- 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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
justinmclean commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1849554303
##
clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java:
##
@@ -551,4 +551,8 @@ public String getUrl() {
// Return the default localhost URL
return DEFAULT_URL;
}
+
+ public static Options getOptions() {
+return options;
+ }
Review Comment:
The org.apache.commons.cli.Options class is specific to a particular
library, so avoiding including it directly in the command signature is best
from a maintainability point of view. Additionally, passing Options makes it
harder to identify which specific options a command requires. It’s better for
code understanding to be explicit about each command's required options. Also,
these changes should follow the style of other commands.
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
xunliu commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1849480387
##
clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java:
##
@@ -551,4 +551,8 @@ public String getUrl() {
// Return the default localhost URL
return DEFAULT_URL;
}
+
+ public static Options getOptions() {
+return options;
+ }
Review Comment:
I think it better to pass `Options` in the function params, because
1. The `Options` is a common class.
2. Different command need different params, we can be unified pass `Options`
params, Otherwise the function will have too many arguments.
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
justinmclean commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1849409986
##
clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java:
##
@@ -551,4 +551,8 @@ public String getUrl() {
// Return the default localhost URL
return DEFAULT_URL;
}
+
+ public static Options getOptions() {
+return options;
+ }
Review Comment:
All other commands extract what they need from options and pass that along.
That way, a command doesn't need to know about the CLI library we use.
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
xunliu commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1849396805
##
clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java:
##
@@ -551,4 +551,8 @@ public String getUrl() {
// Return the default localhost URL
return DEFAULT_URL;
}
+
+ public static Options getOptions() {
+return options;
+ }
Review Comment:
@waukin I think we can also pass `options` in the command function params.
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
justinmclean commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1849060154
##
clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java:
##
@@ -551,4 +551,8 @@ public String getUrl() {
// Return the default localhost URL
return DEFAULT_URL;
}
+
+ public static Options getOptions() {
+return options;
+ }
Review Comment:
I don't think the options should be exposed in this way. Each command is
passed in its constructor the details it needs in order to be able to perform
the command.
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
justinmclean commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1849058732
##
clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java:
##
@@ -93,7 +93,7 @@
public class GravitinoCommandLine {
private final CommandLine line;
- private final Options options;
Review Comment:
Why is this static?
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin commented on PR #5606: URL: https://github.com/apache/gravitino/pull/5606#issuecomment-2485811403 Hi @xunliu, thank you for the update! -- 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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
waukin commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1848424335
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/MetalakeDetails.java:
##
@@ -54,6 +58,10 @@ public void handle() {
return;
}
-System.out.println(metalake + "," + comment);
+List headers = Arrays.asList("metalake", "comment");
+List> rows = new ArrayList<>();
+rows.add(Arrays.asList(metalake, comment));
+TablePrinter tablePrinter = new TablePrinter();
+tablePrinter.print(headers, rows);
Review Comment:
Thank you for pointing this out. I'll update the changes to be optional via
a flag.
--
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]
Re: [PR] [#5506] feat(CLI): Table formatted output [gravitino]
xunliu commented on code in PR #5606:
URL: https://github.com/apache/gravitino/pull/5606#discussion_r1847846692
##
clients/cli/src/main/java/org/apache/gravitino/cli/commands/ListMetalakes.java:
##
@@ -50,13 +51,12 @@ public void handle() {
return;
}
-List metalakeNames = new ArrayList<>();
+List headers = Arrays.asList("metalake");
+List> rows = new ArrayList<>();
for (int i = 0; i < metalakes.length; i++) {
- metalakeNames.add(metalakes[i].name());
+ rows.add(Arrays.asList(metalakes[i].name()));
}
-
-String all = Joiner.on(System.lineSeparator()).join(metalakeNames);
-
-System.out.println(all.toString());
+TablePrinter tablePrinter = new TablePrinter();
+tablePrinter.print(headers, rows);
Review Comment:
hi @justinmclean
I think we only need one format output, why do we needs kept two different
output?
Can you explain?
--
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]
