cnauroth commented on code in PR #50173:
URL: https://github.com/apache/spark/pull/50173#discussion_r1982630520
##########
resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/AmIpFilterSuite.scala:
##########
@@ -162,8 +162,10 @@ class AmIpFilterSuite extends SparkFunSuite {
assert(!filter.getProxyAddresses.isEmpty)
// waiting for configuration update
eventually(timeout(5.seconds), interval(500.millis)) {
- assertThrows[ServletException] {
+ try {
filter.getProxyAddresses.isEmpty
+ } catch {
+ case e: ServletException => true
Review Comment:
It sounds like the intent of this proposed change is to succeed specifically
if there is a `ServletException`, but fail for other exception types. I don't
think we need code changes to achieve this though. Exceptions different from
`ServletException` propagate from the test and cause a test failure.
To confirm this, I just tried introducing a `throw new RuntimeException()`
in my local copy AmIpFilter.java. As I expected, the test failed.
##########
resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/AmIpFilterSuite.scala:
##########
@@ -162,8 +162,10 @@ class AmIpFilterSuite extends SparkFunSuite {
assert(!filter.getProxyAddresses.isEmpty)
// waiting for configuration update
eventually(timeout(5.seconds), interval(500.millis)) {
- assertThrows[ServletException] {
+ try {
filter.getProxyAddresses.isEmpty
+ } catch {
+ case e: ServletException => true
Review Comment:
It sounds like the intent of this proposed change is to succeed specifically
if there is a `ServletException`, but fail for other exception types. I don't
think we need code changes to achieve this though. Exceptions different from
`ServletException` propagate from the test and cause a test failure.
To confirm this, I just tried introducing a `throw new RuntimeException()`
in my local copy of AmIpFilter.java. As I expected, the test failed.
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
