tengqm commented on code in PR #6573:
URL: https://github.com/apache/gravitino/pull/6573#discussion_r1976615393
##########
clients/cli/src/main/java/org/apache/gravitino/cli/commands/ListFilesetProperties.java:
##########
@@ -59,10 +59,15 @@ public ListFilesetProperties(
@Override
public void handle() {
Fileset gFileset = null;
+ GravitinoClient client = null;
Review Comment:
Why this change?
##########
clients/cli/src/main/java/org/apache/gravitino/cli/commands/ListCatalogProperties.java:
##########
@@ -49,10 +49,14 @@ public ListCatalogProperties(CommandContext context, String
metalake, String cat
/** List the properties of a catalog. */
@Override
public void handle() {
- Catalog gCatalog = null;
+ GravitinoClient client = buildClient(metalake);
+
+ if (gCatalog != null) {
+ printProperties(gCatalog.properties());
+ }
+
try {
- GravitinoClient client = buildClient(metalake);
- gCatalog = client.loadCatalog(catalog);
+ Catalog gCatalog = client.loadCatalog(catalog);
} catch (NoSuchMetalakeException err) {
Review Comment:
Oh No. This code won't work.
Checking gCatalog on line 54 before it is declared on line 59?
Throw NoSuchMetalakeException while metalake is used on line 52?
##########
clients/cli/src/main/java/org/apache/gravitino/cli/commands/TableAudit.java:
##########
@@ -53,10 +57,22 @@ public void handle() {
try {
NameIdentifier name = NameIdentifier.of(schema, table);
gTable = tableCatalog().loadTable(name);
+
+ } catch (NoSuchMetalakeException err) {
+ exitWithError(ErrorMessages.UNKNOWN_METALAKE);
+ } catch (NoSuchCatalogException err) {
+ exitWithError(ErrorMessages.UNKNOWN_CATALOG);
+ } catch (NoSuchSchemaException err) {
+ exitWithError(ErrorMessages.UNKNOWN_SCHEMA);
} catch (Exception exp) {
exitWithError(exp.getMessage());
}
+ if (gTable == null) {
+ exitWithError("Failed to retrieve table details.");
+ return;
Review Comment:
I don't have a strong opinion about this additional `return` line, though
I'm more leaning towards having it (no harm + code readability).
However, when we decide to add these `return`s, we need to be consistent.
For example, line 68 doesn't have a following `return` statement, this would
confuse others.
--
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: commits-unsubscr...@gravitino.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
