You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@bahir.apache.org by GitBox <gi...@apache.org> on 2020/07/18 15:53:51 UTC
[GitHub] [bahir] abhishekd0907 opened a new pull request #99: [BAHIR-239] Fixing bugs in SQSClient
abhishekd0907 opened a new pull request #99:
URL: https://github.com/apache/bahir/pull/99
## What Changes were proposed in this pull request?
This pull request fixes the following two issues:
1. Incomplete error message shows up when SQSClient fails to be created.
`org.apache.spark.SparkException: Error occured while creating Amazon SQS Client null`
2. AWS region is not honoured when authentication mode is keys.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [bahir] abhishekd0907 commented on a change in pull request #99: [BAHIR-239] Fixing bugs in SQSClient
Posted by GitBox <gi...@apache.org>.
abhishekd0907 commented on a change in pull request #99:
URL: https://github.com/apache/bahir/pull/99#discussion_r460488174
##########
File path: sql-streaming-sqs/src/main/scala/org/apache/spark/sql/streaming/sqs/SqsClient.scala
##########
@@ -220,11 +220,12 @@ class SqsClient(sourceOptions: SqsSourceOptions,
.standard()
.withClientConfiguration(new ClientConfiguration().withMaxConnections(maxConnections))
.withCredentials(instanceProfileCredentialsProvider)
+ .withRegion(region)
.build()
}
} catch {
case e: Exception =>
- throw new SparkException(s"Error occured while creating Amazon SQS Client ${e.getMessage}")
+ throw new SparkException(s"Error occured while creating Amazon SQS Client", e)
Review comment:
`getMessage` returns null for some AWS exceptions. Better way to show stacktrace is to pass the cause to SparkException.
Example:
```
throw new SparkException("outer exception", new ArrayIndexOutOfBoundsException("inner message")) =>
org.apache.spark.SparkException: outer exception
... 49 elided
Caused by: java.lang.ArrayIndexOutOfBoundsException: inner message
... 49 more
```
How old code worked:
```
val e = new ArrayIndexOutOfBoundsException()
throw new SparkException(s"outer exception ${e.getMessage}") =>
org.apache.spark.SparkException:outer exception null
... 49 elided
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [bahir] lresende merged pull request #99: [BAHIR-239] Fixing bugs in SQSClient
Posted by GitBox <gi...@apache.org>.
lresende merged pull request #99:
URL: https://github.com/apache/bahir/pull/99
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [bahir] lresende commented on a change in pull request #99: [BAHIR-239] Fixing bugs in SQSClient
Posted by GitBox <gi...@apache.org>.
lresende commented on a change in pull request #99:
URL: https://github.com/apache/bahir/pull/99#discussion_r457486342
##########
File path: sql-streaming-sqs/src/main/scala/org/apache/spark/sql/streaming/sqs/SqsClient.scala
##########
@@ -220,11 +220,12 @@ class SqsClient(sourceOptions: SqsSourceOptions,
.standard()
.withClientConfiguration(new ClientConfiguration().withMaxConnections(maxConnections))
.withCredentials(instanceProfileCredentialsProvider)
+ .withRegion(region)
.build()
}
} catch {
case e: Exception =>
- throw new SparkException(s"Error occured while creating Amazon SQS Client ${e.getMessage}")
+ throw new SparkException(s"Error occured while creating Amazon SQS Client", e)
Review comment:
Adding the message is good, but did you intended to remove the "cause" on the 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:
users@infra.apache.org
[GitHub] [bahir] abhishekd0907 commented on pull request #99: [BAHIR-239] Fixing bugs in SQSClient
Posted by GitBox <gi...@apache.org>.
abhishekd0907 commented on pull request #99:
URL: https://github.com/apache/bahir/pull/99#issuecomment-660612648
@lresende can you please review this change?
----------------------------------------------------------------
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:
users@infra.apache.org