You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2021/10/18 15:54:33 UTC
[GitHub] [hive] zabetak opened a new pull request #2726: HIVE-25618: Stack trace is difficult to find when qtest fails during setup/teardown
zabetak opened a new pull request #2726:
URL: https://github.com/apache/hive/pull/2726
### What changes were proposed in this pull request?
Remove the try, catch, print, and fail pattern in CLI drivers letting the exception to propagate.
### Why are the changes needed?
1. It is easier to find the original stack trace causing the failure without looking into multiple places.
2. It makes the code more readable.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Manual and existing tests
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak commented on a change in pull request #2726: HIVE-25618: Stack trace is difficult to find when qtest fails during setup/teardown
Posted by GitBox <gi...@apache.org>.
zabetak commented on a change in pull request #2726:
URL: https://github.com/apache/hive/pull/2726#discussion_r731769916
##########
File path: itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliAdapter.java
##########
@@ -59,10 +59,10 @@ public CliAdapter(AbstractCliConfig cliConfig) {
public abstract void beforeClass() throws Exception;
// HIVE-14444 pending rename: before
Review comment:
I prefer doing this in a separate JIRA/commit to keep this issue more self-contained.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak closed pull request #2726: HIVE-25618: Stack trace is difficult to find when qtest fails during setup/teardown
Posted by GitBox <gi...@apache.org>.
zabetak closed pull request #2726:
URL: https://github.com/apache/hive/pull/2726
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kgyrtkirk commented on a change in pull request #2726: HIVE-25618: Stack trace is difficult to find when qtest fails during setup/teardown
Posted by GitBox <gi...@apache.org>.
kgyrtkirk commented on a change in pull request #2726:
URL: https://github.com/apache/hive/pull/2726#discussion_r731753660
##########
File path: itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliAdapter.java
##########
@@ -59,10 +59,10 @@ public CliAdapter(AbstractCliConfig cliConfig) {
public abstract void beforeClass() throws Exception;
// HIVE-14444 pending rename: before
Review comment:
you may also rename these methods....
##########
File path: itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreCliDriver.java
##########
@@ -67,91 +67,60 @@ public void beforeClass() {
String cleanupScript = cliConfig.getCleanupScript();
Set<QTestExternalDB> externalDBs = cliConfig.getExternalDBs();
- try {
- qt = new ElapsedTimeLoggingWrapper<QTestUtil>() {
- @Override
- public QTestUtil invokeInternal() throws Exception {
- return new QTestUtil(
- QTestArguments.QTestArgumentsBuilder.instance()
- .withOutDir(cliConfig.getResultsDir())
- .withLogDir(cliConfig.getLogDir())
- .withClusterType(miniMR)
- .withConfDir(hiveConfDir)
- .withInitScript(initScript)
- .withCleanupScript(cleanupScript)
- .withExternalDBs(externalDBs)
- .withLlapIo(true)
- .withFsType(cliConfig.getFsType())
- .build());
- }
- }.invoke("QtestUtil instance created", LOG, true);
- } catch (Exception e) {
- System.err.println("Exception: " + e.getMessage());
- e.printStackTrace();
- System.err.flush();
- throw new RuntimeException("Unexpected exception in static initialization", e);
- }
+ qt = new ElapsedTimeLoggingWrapper<QTestUtil>() {
+ @Override
+ public QTestUtil invokeInternal() throws Exception {
+ return new QTestUtil(
+ QTestArguments.QTestArgumentsBuilder.instance()
+ .withOutDir(cliConfig.getResultsDir())
+ .withLogDir(cliConfig.getLogDir())
+ .withClusterType(miniMR)
+ .withConfDir(hiveConfDir)
+ .withInitScript(initScript)
+ .withCleanupScript(cleanupScript)
+ .withExternalDBs(externalDBs)
+ .withLlapIo(true)
+ .withFsType(cliConfig.getFsType())
+ .build());
+ }
+ }.invoke("QtestUtil instance created", LOG, true);
}
@Override
@Before
- public void setUp() {
- try {
- new ElapsedTimeLoggingWrapper<Void>() {
- @Override
- public Void invokeInternal() throws Exception {
- qt.newSession();
- return null;
- }
- }.invoke("PerTestSetup done.", LOG, false);
-
- } catch (Exception e) {
- System.err.println("Exception: " + e.getMessage());
- e.printStackTrace();
- System.err.flush();
- fail("Unexpected exception in setup");
- }
+ public void setUp() throws Exception {
+ new ElapsedTimeLoggingWrapper<Void>() {
+ @Override
+ public Void invokeInternal() throws Exception {
+ qt.newSession();
+ return null;
+ }
+ }.invoke("PerTestSetup done.", LOG, false);
}
@Override
@After
- public void tearDown() {
- try {
- new ElapsedTimeLoggingWrapper<Void>() {
- @Override
- public Void invokeInternal() throws Exception {
- qt.clearPostTestEffects();
- qt.clearTestSideEffects();
- return null;
- }
- }.invoke("PerTestTearDown done.", LOG, false);
-
- } catch (Exception e) {
- System.err.println("Exception: " + e.getMessage());
- e.printStackTrace();
- System.err.flush();
- fail("Unexpected exception in tearDown");
- }
+ public void tearDown() throws Exception {
+ new ElapsedTimeLoggingWrapper<Void>() {
Review comment:
I think these LogginWrapper things doesnt add much value - if we want something like this we could implement it in a different manner...
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak commented on a change in pull request #2726: HIVE-25618: Stack trace is difficult to find when qtest fails during setup/teardown
Posted by GitBox <gi...@apache.org>.
zabetak commented on a change in pull request #2726:
URL: https://github.com/apache/hive/pull/2726#discussion_r731772394
##########
File path: itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreCliDriver.java
##########
@@ -67,91 +67,60 @@ public void beforeClass() {
String cleanupScript = cliConfig.getCleanupScript();
Set<QTestExternalDB> externalDBs = cliConfig.getExternalDBs();
- try {
- qt = new ElapsedTimeLoggingWrapper<QTestUtil>() {
- @Override
- public QTestUtil invokeInternal() throws Exception {
- return new QTestUtil(
- QTestArguments.QTestArgumentsBuilder.instance()
- .withOutDir(cliConfig.getResultsDir())
- .withLogDir(cliConfig.getLogDir())
- .withClusterType(miniMR)
- .withConfDir(hiveConfDir)
- .withInitScript(initScript)
- .withCleanupScript(cleanupScript)
- .withExternalDBs(externalDBs)
- .withLlapIo(true)
- .withFsType(cliConfig.getFsType())
- .build());
- }
- }.invoke("QtestUtil instance created", LOG, true);
- } catch (Exception e) {
- System.err.println("Exception: " + e.getMessage());
- e.printStackTrace();
- System.err.flush();
- throw new RuntimeException("Unexpected exception in static initialization", e);
- }
+ qt = new ElapsedTimeLoggingWrapper<QTestUtil>() {
+ @Override
+ public QTestUtil invokeInternal() throws Exception {
+ return new QTestUtil(
+ QTestArguments.QTestArgumentsBuilder.instance()
+ .withOutDir(cliConfig.getResultsDir())
+ .withLogDir(cliConfig.getLogDir())
+ .withClusterType(miniMR)
+ .withConfDir(hiveConfDir)
+ .withInitScript(initScript)
+ .withCleanupScript(cleanupScript)
+ .withExternalDBs(externalDBs)
+ .withLlapIo(true)
+ .withFsType(cliConfig.getFsType())
+ .build());
+ }
+ }.invoke("QtestUtil instance created", LOG, true);
}
@Override
@Before
- public void setUp() {
- try {
- new ElapsedTimeLoggingWrapper<Void>() {
- @Override
- public Void invokeInternal() throws Exception {
- qt.newSession();
- return null;
- }
- }.invoke("PerTestSetup done.", LOG, false);
-
- } catch (Exception e) {
- System.err.println("Exception: " + e.getMessage());
- e.printStackTrace();
- System.err.flush();
- fail("Unexpected exception in setup");
- }
+ public void setUp() throws Exception {
+ new ElapsedTimeLoggingWrapper<Void>() {
+ @Override
+ public Void invokeInternal() throws Exception {
+ qt.newSession();
+ return null;
+ }
+ }.invoke("PerTestSetup done.", LOG, false);
}
@Override
@After
- public void tearDown() {
- try {
- new ElapsedTimeLoggingWrapper<Void>() {
- @Override
- public Void invokeInternal() throws Exception {
- qt.clearPostTestEffects();
- qt.clearTestSideEffects();
- return null;
- }
- }.invoke("PerTestTearDown done.", LOG, false);
-
- } catch (Exception e) {
- System.err.println("Exception: " + e.getMessage());
- e.printStackTrace();
- System.err.flush();
- fail("Unexpected exception in tearDown");
- }
+ public void tearDown() throws Exception {
+ new ElapsedTimeLoggingWrapper<Void>() {
Review comment:
I totally agree. I will log a separate JIRA again for keeping things more self-contained and giving a heads-up to others who may have a different opinion.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org