[GitHub] [hudi] hddong commented on a change in pull request #1770: [HUDI-708]Add temps show and unit test for TempViewCommand
hddong commented on a change in pull request #1770:
URL: https://github.com/apache/hudi/pull/1770#discussion_r458821046
##
File path:
hudi-cli/src/main/java/org/apache/hudi/cli/utils/SparkTempViewProvider.java
##
@@ -101,6 +101,17 @@ public void runQuery(String sqlText) {
}
}
+ @Override
+ public void showAllViews() {
+try {
+ sqlContext.sql("SHOW TABLES").show(Integer.MAX_VALUE, false);
+} catch (Throwable ex) {
+ // log full stack trace and rethrow. Without this its difficult to debug
failures, if any
+ LOG.error("unable to initialize spark context ", ex);
Review comment:
> IMO, this error description is not correct, right? The throwable not
only covers the initialization of the spark context
Yes, change it to `unable to get all views`.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [hudi] hddong commented on a change in pull request #1770: [HUDI-708]Add temps show and unit test for TempViewCommand
hddong commented on a change in pull request #1770:
URL: https://github.com/apache/hudi/pull/1770#discussion_r458817753
##
File path:
hudi-cli/src/main/java/org/apache/hudi/cli/commands/TempViewCommand.java
##
@@ -20,36 +20,55 @@
import org.apache.hudi.cli.HoodieCLI;
+import org.apache.hudi.exception.HoodieException;
import org.springframework.shell.core.CommandMarker;
import org.springframework.shell.core.annotation.CliCommand;
import org.springframework.shell.core.annotation.CliOption;
import org.springframework.stereotype.Component;
-import java.io.IOException;
-
/**
* CLI command to query/delete temp views.
*/
@Component
public class TempViewCommand implements CommandMarker {
- private static final String EMPTY_STRING = "";
+ public static final String QUERY_SUCCESS = "Query ran successfully!";
+ public static final String QUERY_FAIL = "Query ran failed!";
+ public static final String SHOW_SUCCESS = "Show all views name
successfully!";
- @CliCommand(value = "temp_query", help = "query against created temp view")
+ @CliCommand(value = {"temp_query", "temp query"}, help = "query against
created temp view")
public String query(
- @CliOption(key = {"sql"}, mandatory = true, help = "select query to
run against view") final String sql)
- throws IOException {
+ @CliOption(key = {"sql"}, mandatory = true, help = "select query to
run against view") final String sql) {
+
+try {
+ HoodieCLI.getTempViewProvider().runQuery(sql);
+ return QUERY_SUCCESS;
+} catch (HoodieException ex) {
+ return QUERY_FAIL;
Review comment:
`runQuery` `showAllViews ` and `deleteTable` had log the detailed
exception.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [hudi] hddong commented on a change in pull request #1770: [HUDI-708]Add temps show and unit test for TempViewCommand
hddong commented on a change in pull request #1770:
URL: https://github.com/apache/hudi/pull/1770#discussion_r458817753
##
File path:
hudi-cli/src/main/java/org/apache/hudi/cli/commands/TempViewCommand.java
##
@@ -20,36 +20,55 @@
import org.apache.hudi.cli.HoodieCLI;
+import org.apache.hudi.exception.HoodieException;
import org.springframework.shell.core.CommandMarker;
import org.springframework.shell.core.annotation.CliCommand;
import org.springframework.shell.core.annotation.CliOption;
import org.springframework.stereotype.Component;
-import java.io.IOException;
-
/**
* CLI command to query/delete temp views.
*/
@Component
public class TempViewCommand implements CommandMarker {
- private static final String EMPTY_STRING = "";
+ public static final String QUERY_SUCCESS = "Query ran successfully!";
+ public static final String QUERY_FAIL = "Query ran failed!";
+ public static final String SHOW_SUCCESS = "Show all views name
successfully!";
- @CliCommand(value = "temp_query", help = "query against created temp view")
+ @CliCommand(value = {"temp_query", "temp query"}, help = "query against
created temp view")
public String query(
- @CliOption(key = {"sql"}, mandatory = true, help = "select query to
run against view") final String sql)
- throws IOException {
+ @CliOption(key = {"sql"}, mandatory = true, help = "select query to
run against view") final String sql) {
+
+try {
+ HoodieCLI.getTempViewProvider().runQuery(sql);
+ return QUERY_SUCCESS;
+} catch (HoodieException ex) {
+ return QUERY_FAIL;
Review comment:
`runQuery` had log the detailed exception.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [hudi] hddong commented on a change in pull request #1770: [HUDI-708]Add temps show and unit test for TempViewCommand
hddong commented on a change in pull request #1770:
URL: https://github.com/apache/hudi/pull/1770#discussion_r458817019
##
File path: hudi-cli/src/main/java/org/apache/hudi/cli/HoodieCLI.java
##
@@ -115,4 +115,16 @@ public static synchronized TempViewProvider
getTempViewProvider() {
return tempViewProvider;
}
+ /**
+ * Close tempViewProvider.
+ *
+ * For test, avoid multiple SparkContexts.
+ */
+ public static synchronized void closeTempViewProvider() {
Review comment:
+1, @VisibleForTesting can be useful.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
