arjun4084346 commented on a change in pull request #3100:
URL: https://github.com/apache/incubator-gobblin/pull/3100#discussion_r487198150
##########
File path:
gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinHelixTask.java
##########
@@ -213,9 +213,17 @@ private Integer getPartitionForHelixTask(TaskDriver
taskDriver) {
@Override
public void cancel() {
- log.info("Gobblin helix task cancellation invoked.");
+ log.info("Gobblin helix task cancellation invoked for jobId {}.", jobId);
if (this.task != null ) {
- this.task.cancel();
+ try {
+ this.task.cancel();
+ log.info("Gobblin helix task cancellation completed for jobId {}.",
jobId);
+ } catch (Throwable t) {
+ log.info("Gobblin helix task cancellation for jobId {} failed with
exception.", jobId, t);
Review comment:
I do not intend to just catch and throw the Throwable. I wanted to log
the exception in gobblin's log. This method is called from helix class and I do
not want to depend on helix code on exception handling.
##########
File path:
gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinHelixTask.java
##########
@@ -213,9 +213,17 @@ private Integer getPartitionForHelixTask(TaskDriver
taskDriver) {
@Override
public void cancel() {
- log.info("Gobblin helix task cancellation invoked.");
+ log.info("Gobblin helix task cancellation invoked for jobId {}.", jobId);
if (this.task != null ) {
- this.task.cancel();
+ try {
+ this.task.cancel();
+ log.info("Gobblin helix task cancellation completed for jobId {}.",
jobId);
+ } catch (Throwable t) {
+ log.info("Gobblin helix task cancellation for jobId {} failed with
exception.", jobId, t);
+ throw new RuntimeException("Gobblin helix task cancellation for jobId
" + jobId + " failed with exception.", t);
Review comment:
the method signature does not throw any exception. RuntimeException is
Unchecked exception and can be thrown without changing the signature. Throwable
is not Unchecked exception.
##########
File path:
gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinHelixTask.java
##########
@@ -213,9 +213,17 @@ private Integer getPartitionForHelixTask(TaskDriver
taskDriver) {
@Override
public void cancel() {
- log.info("Gobblin helix task cancellation invoked.");
+ log.info("Gobblin helix task cancellation invoked for jobId {}.", jobId);
if (this.task != null ) {
- this.task.cancel();
+ try {
+ this.task.cancel();
+ log.info("Gobblin helix task cancellation completed for jobId {}.",
jobId);
+ } catch (Throwable t) {
+ log.info("Gobblin helix task cancellation for jobId {} failed with
exception.", jobId, t);
Review comment:
I do not intend to just catch and throw the Throwable. I wanted to log
the exception in gobblin's log. This method is called from helix class and I do
not want to depend on helix code on exception handling.
##########
File path:
gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinHelixTask.java
##########
@@ -213,9 +213,17 @@ private Integer getPartitionForHelixTask(TaskDriver
taskDriver) {
@Override
public void cancel() {
- log.info("Gobblin helix task cancellation invoked.");
+ log.info("Gobblin helix task cancellation invoked for jobId {}.", jobId);
if (this.task != null ) {
- this.task.cancel();
+ try {
+ this.task.cancel();
+ log.info("Gobblin helix task cancellation completed for jobId {}.",
jobId);
+ } catch (Throwable t) {
+ log.info("Gobblin helix task cancellation for jobId {} failed with
exception.", jobId, t);
+ throw new RuntimeException("Gobblin helix task cancellation for jobId
" + jobId + " failed with exception.", t);
Review comment:
the method signature does not throw any exception. RuntimeException is
Unchecked exception and can be thrown without changing the signature. Throwable
is not Unchecked 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:
[email protected]
