You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/01/25 13:27:24 UTC
[GitHub] [spark] LuciferYang opened a new pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
LuciferYang opened a new pull request #31323:
URL: https://github.com/apache/spark/pull/31323
### What changes were proposed in this pull request?
Using a function like `.mkString` or `.getLines` directly on a `scala.io.Source` opened by `fromFile`, `fromURL`, `fromURI ` will leak the underlying file handle, this pr use the `Utils.tryWithResource` method wrap the `BufferedSource` to ensure these `BufferedSource` closed.
### Why are the changes needed?
Avoid file handle leak.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Pass the Jenkins or GitHub Action
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564198841
##########
File path: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaTestUtils.scala
##########
@@ -167,29 +167,31 @@ class KafkaTestUtils(
* In this method we rewrite krb5.conf to make kdc and client use the same enctypes
*/
private def rewriteKrb5Conf(): Unit = {
- val krb5Conf = Source.fromFile(kdc.getKrb5conf, "UTF-8").getLines()
- var rewritten = false
- val addedConfig =
- addedKrb5Config("default_tkt_enctypes", "aes128-cts-hmac-sha1-96") +
- addedKrb5Config("default_tgs_enctypes", "aes128-cts-hmac-sha1-96")
- val rewriteKrb5Conf = krb5Conf.map(s =>
- if (s.contains("libdefaults")) {
- rewritten = true
- s + addedConfig
+ Utils.tryWithResource(Source.fromFile(kdc.getKrb5conf, "UTF-8")) { source =>
Review comment:
```
sbt.ForkMain$ForkError: java.io.IOException: Stream Closed
at java.io.FileInputStream.readBytes(Native Method)
at java.io.FileInputStream.read(FileInputStream.java:255)
at sun.nio.cs.StreamDecoder.readBytes(StreamDecoder.java:284)
at sun.nio.cs.StreamDecoder.implRead(StreamDecoder.java:326)
at sun.nio.cs.StreamDecoder.read(StreamDecoder.java:178)
at java.io.InputStreamReader.read(InputStreamReader.java:184)
at java.io.BufferedReader.fill(BufferedReader.java:161)
at java.io.BufferedReader.readLine(BufferedReader.java:324)
at java.io.BufferedReader.readLine(BufferedReader.java:389)
at scala.io.BufferedSource$BufferedLineIterator.hasNext(BufferedSource.scala:74)
at scala.collection.Iterator$$anon$10.hasNext(Iterator.scala:460)
at scala.collection.Iterator$$anon$12.hasNext(Iterator.scala:513)
at scala.collection.Iterator.foreach(Iterator.scala:943)
at scala.collection.Iterator.foreach$(Iterator.scala:943)
at scala.collection.AbstractIterator.foreach(Iterator.scala:1431)
at scala.collection.TraversableOnce.addString(TraversableOnce.scala:424)
at scala.collection.TraversableOnce.addString$(TraversableOnce.scala:407)
at scala.collection.AbstractIterator.addString(Iterator.scala:1431)
at scala.collection.TraversableOnce.mkString(TraversableOnce.scala:377)
at scala.collection.TraversableOnce.mkString$(TraversableOnce.scala:376)
at scala.collection.AbstractIterator.mkString(Iterator.scala:1431)
at scala.collection.TraversableOnce.mkString(TraversableOnce.scala:379)
at scala.collection.TraversableOnce.mkString$(TraversableOnce.scala:379)
at scala.collection.AbstractIterator.mkString(Iterator.scala:1431)
at org.apache.spark.sql.kafka010.KafkaTestUtils.rewriteKrb5Conf(KafkaTestUtils.scala:182)
at org.apache.spark.sql.kafka010.KafkaTestUtils.setUpMiniKdc(KafkaTestUtils.scala:162)
at org.apache.spark.sql.kafka010.KafkaTestUtils.setup(KafkaTestUtils.scala:304)
at org.apache.spark.sql.kafka010.KafkaDelegationTokenSuite.beforeAll(KafkaDelegationTokenSuite.scala:49)
```
`Utils.tryWithResource(Source.fromFile(kdc.getKrb5conf, "UTF-8")) (_.getLines())` seems return a lazy iter, so there expansion` Utils.tryWithResource` scope to avoid `java.io.IOException: Stream Closed`.
Or we need to materialize `Utils.tryWithResource(Source.fromFile(kdc.getKrb5conf, "UTF-8")) (_.getLines())` like
`Utils.tryWithResource(Source.fromFile(kdc.getKrb5conf, "UTF-8")) (_.getLines().toList)`
Which one would you choose @dongjoon-hyun
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767324706
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39064/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766815500
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766862509
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39031/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang edited a comment on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang edited a comment on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767485739
@HyukjinKwon
- 3.0: https://github.com/apache/spark/pull/31344
- 2.4: https://github.com/apache/spark/pull/31345
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564233512
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala
##########
@@ -36,8 +37,9 @@ case class QueryExecutionTestRecord(
class QueryExecutionSuite extends SharedSparkSession {
import testImplicits._
- def checkDumpedPlans(path: String, expected: Int): Unit = {
- assert(Source.fromFile(path).getLines.toList
+ def checkDumpedPlans(path: String, expected: Int): Unit = Utils.tryWithResource(
+ Source.fromFile(path)) { source =>
Review comment:
@HyukjinKwon line 41 should have 4 space or 2 Space ? thx ~
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r563757768
##########
File path: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaSourceOffsetSuite.scala
##########
@@ -99,7 +100,9 @@ class KafkaSourceOffsetSuite extends OffsetSuite with SharedSparkSession {
private def readFromResource(file: String): SerializedOffset = {
import scala.io.Source
val input = getClass.getResource(s"/$file").toURI
- val str = Source.fromFile(input).mkString
- SerializedOffset(str)
+ Utils.tryWithResource(Source.fromFile(input)) { source =>
+ val str = source.mkString
Review comment:
removed
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767485739
@HyukjinKwon
- 3.0: https://github.com/apache/spark/pull/31344
- 2.4:https://github.com/apache/spark/pull/31345
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767365549
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39069/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767297824
LGTM otherwise
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564234628
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalogVersionsSuite.scala
##########
@@ -243,12 +243,14 @@ object PROCESS_TABLES extends QueryTest with SQLTestUtils {
val testingVersions: Seq[String] = {
import scala.io.Source
val versions: Seq[String] = try {
Review comment:
done ~
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala
##########
@@ -37,21 +38,23 @@ class QueryExecutionSuite extends SharedSparkSession {
import testImplicits._
def checkDumpedPlans(path: String, expected: Int): Unit = {
Review comment:
done
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564214936
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala
##########
@@ -37,21 +38,23 @@ class QueryExecutionSuite extends SharedSparkSession {
import testImplicits._
def checkDumpedPlans(path: String, expected: Int): Unit = {
Review comment:
This too, can you fix it to:
```scala
def checkDumpedPlans(path: String, expected: Int): Unit = Utils.tryWithResource(
Source.fromFile(path)) { source =>
// blah blah
}
```
?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564214748
##########
File path: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaTestUtils.scala
##########
@@ -167,29 +167,31 @@ class KafkaTestUtils(
* In this method we rewrite krb5.conf to make kdc and client use the same enctypes
*/
private def rewriteKrb5Conf(): Unit = {
Review comment:
@LuciferYang, can you fix this line to `Utils.tryWithResource(Source.fromFile(kdc.getKrb5conf, "UTF-8")) { krb5Conf =>`? that will reduce the diff a lot.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767383029
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564233512
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala
##########
@@ -36,8 +37,9 @@ case class QueryExecutionTestRecord(
class QueryExecutionSuite extends SharedSparkSession {
import testImplicits._
- def checkDumpedPlans(path: String, expected: Int): Unit = {
- assert(Source.fromFile(path).getLines.toList
+ def checkDumpedPlans(path: String, expected: Int): Unit = Utils.tryWithResource(
+ Source.fromFile(path)) { source =>
Review comment:
@HyukjinKwon 4 space or 2 Space ? thx ~
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767334487
**[Test build #134483 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134483/testReport)** for PR 31323 at commit [`245d844`](https://github.com/apache/spark/commit/245d8446e831043a140b8c16a31a9710328a67a7).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jaceklaskowski commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
jaceklaskowski commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r563951651
##########
File path: core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
##########
@@ -327,22 +328,25 @@ class MasterSuite extends SparkFunSuite
val masterUrl = s"http://localhost:${localCluster.masterWebUIPort}"
try {
eventually(timeout(5.seconds), interval(100.milliseconds)) {
- val json = Source.fromURL(s"$masterUrl/json").getLines().mkString("\n")
+ val json = Utils
+ .tryWithResource(Source.fromURL(s"$masterUrl/json"))(_.getLines().mkString("\n"))
val JArray(workers) = (parse(json) \ "workers")
workers.size should be (2)
workers.foreach { workerSummaryJson =>
val JString(workerWebUi) = workerSummaryJson \ "webuiaddress"
- val workerResponse = parse(Source.fromURL(s"${workerWebUi}/json")
- .getLines().mkString("\n"))
+ val workerResponse = parse(Utils
+ .tryWithResource(Source.fromURL(s"$workerWebUi/json"))(_.getLines().mkString("\n")))
Review comment:
Are the `getLines()` and `\n` actually needed? Review other changes.
##########
File path: core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
##########
@@ -327,22 +328,25 @@ class MasterSuite extends SparkFunSuite
val masterUrl = s"http://localhost:${localCluster.masterWebUIPort}"
try {
eventually(timeout(5.seconds), interval(100.milliseconds)) {
- val json = Source.fromURL(s"$masterUrl/json").getLines().mkString("\n")
+ val json = Utils
+ .tryWithResource(Source.fromURL(s"$masterUrl/json"))(_.getLines().mkString("\n"))
Review comment:
Are the `getLines()` and `\n` actually needed?
##########
File path: core/src/test/scala/org/apache/spark/SparkContextSuite.scala
##########
@@ -376,7 +377,9 @@ class SparkContextSuite extends SparkFunSuite with LocalSparkContext with Eventu
sc.addFile(file1.getAbsolutePath)
def getAddedFileContents(): String = {
sc.parallelize(Seq(0)).map { _ =>
- scala.io.Source.fromFile(SparkFiles.get("file")).mkString
+ Utils.tryWithResource(Source.fromFile(SparkFiles.get("file"))) { source =>
+ source.mkString
Review comment:
`_.mkString`?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766815500
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766891693
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jaceklaskowski commented on a change in pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
jaceklaskowski commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r563724184
##########
File path: core/src/test/scala/org/apache/spark/deploy/LogUrlsStandaloneSuite.scala
##########
@@ -43,7 +43,9 @@ class LogUrlsStandaloneSuite extends SparkFunSuite with LocalSparkContext {
assert(info.logUrlMap.nonEmpty)
// Browse to each URL to check that it's valid
info.logUrlMap.foreach { case (logType, logUrl) =>
- val html = Source.fromURL(logUrl).mkString
+ val html = Utils.tryWithResource(Source.fromURL(logUrl)) { source =>
+ source.mkString
Review comment:
nit: `_.mkString`
##########
File path: core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
##########
@@ -327,22 +328,31 @@ class MasterSuite extends SparkFunSuite
val masterUrl = s"http://localhost:${localCluster.masterWebUIPort}"
try {
eventually(timeout(5.seconds), interval(100.milliseconds)) {
- val json = Source.fromURL(s"$masterUrl/json").getLines().mkString("\n")
+ val json = Utils.tryWithResource(
+ Source.fromURL(s"$masterUrl/json")) { source =>
+ source.getLines().mkString("\n")
Review comment:
nit: Indent (and perhaps `_.getLines...` too?)
##########
File path: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaSourceOffsetSuite.scala
##########
@@ -99,7 +100,9 @@ class KafkaSourceOffsetSuite extends OffsetSuite with SharedSparkSession {
private def readFromResource(file: String): SerializedOffset = {
import scala.io.Source
val input = getClass.getResource(s"/$file").toURI
- val str = Source.fromFile(input).mkString
- SerializedOffset(str)
+ Utils.tryWithResource(Source.fromFile(input)) { source =>
+ val str = source.mkString
Review comment:
Any reason for this `str` value?
##########
File path: core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
##########
@@ -327,22 +328,31 @@ class MasterSuite extends SparkFunSuite
val masterUrl = s"http://localhost:${localCluster.masterWebUIPort}"
try {
eventually(timeout(5.seconds), interval(100.milliseconds)) {
- val json = Source.fromURL(s"$masterUrl/json").getLines().mkString("\n")
+ val json = Utils.tryWithResource(
+ Source.fromURL(s"$masterUrl/json")) { source =>
+ source.getLines().mkString("\n")
+ }
val JArray(workers) = (parse(json) \ "workers")
workers.size should be (2)
workers.foreach { workerSummaryJson =>
val JString(workerWebUi) = workerSummaryJson \ "webuiaddress"
- val workerResponse = parse(Source.fromURL(s"${workerWebUi}/json")
- .getLines().mkString("\n"))
+ val workerResponse = Utils.tryWithResource(
+ Source.fromURL(s"$workerWebUi/json")) { source =>
+ parse(source.getLines().mkString("\n"))
Review comment:
nit: Indent
##########
File path: examples/src/main/scala/org/apache/spark/examples/DFSReadWriteTest.scala
##########
@@ -46,7 +47,10 @@ object DFSReadWriteTest {
private val NPARAMS = 2
private def readFile(filename: String): List[String] = {
- val lineIter: Iterator[String] = fromFile(filename).getLines()
+ val lineIter: Iterator[String] =
Review comment:
While we're at it...
```
Utils.tryWithResource(fromFile(filename))(_.getLines).toList
```
##########
File path: core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
##########
@@ -327,22 +328,25 @@ class MasterSuite extends SparkFunSuite
val masterUrl = s"http://localhost:${localCluster.masterWebUIPort}"
try {
eventually(timeout(5.seconds), interval(100.milliseconds)) {
- val json = Source.fromURL(s"$masterUrl/json").getLines().mkString("\n")
+ val json = Utils
+ .tryWithResource(Source.fromURL(s"$masterUrl/json"))(_.getLines().mkString("\n"))
val JArray(workers) = (parse(json) \ "workers")
workers.size should be (2)
workers.foreach { workerSummaryJson =>
val JString(workerWebUi) = workerSummaryJson \ "webuiaddress"
- val workerResponse = parse(Source.fromURL(s"${workerWebUi}/json")
- .getLines().mkString("\n"))
+ val workerResponse = parse(Utils
+ .tryWithResource(Source.fromURL(s"$workerWebUi/json"))(_.getLines().mkString("\n")))
Review comment:
Are the `getLines()` and `\n` actually needed? Review other changes.
##########
File path: core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
##########
@@ -327,22 +328,25 @@ class MasterSuite extends SparkFunSuite
val masterUrl = s"http://localhost:${localCluster.masterWebUIPort}"
try {
eventually(timeout(5.seconds), interval(100.milliseconds)) {
- val json = Source.fromURL(s"$masterUrl/json").getLines().mkString("\n")
+ val json = Utils
+ .tryWithResource(Source.fromURL(s"$masterUrl/json"))(_.getLines().mkString("\n"))
Review comment:
Are the `getLines()` and `\n` actually needed?
##########
File path: core/src/test/scala/org/apache/spark/SparkContextSuite.scala
##########
@@ -376,7 +377,9 @@ class SparkContextSuite extends SparkFunSuite with LocalSparkContext with Eventu
sc.addFile(file1.getAbsolutePath)
def getAddedFileContents(): String = {
sc.parallelize(Seq(0)).map { _ =>
- scala.io.Source.fromFile(SparkFiles.get("file")).mkString
+ Utils.tryWithResource(Source.fromFile(SparkFiles.get("file"))) { source =>
+ source.mkString
Review comment:
`_.mkString`?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766891693
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766934128
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564233512
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala
##########
@@ -36,8 +37,9 @@ case class QueryExecutionTestRecord(
class QueryExecutionSuite extends SharedSparkSession {
import testImplicits._
- def checkDumpedPlans(path: String, expected: Int): Unit = {
- assert(Source.fromFile(path).getLines.toList
+ def checkDumpedPlans(path: String, expected: Int): Unit = Utils.tryWithResource(
+ Source.fromFile(path)) { source =>
Review comment:
@HyukjinKwon 4 space or 2 Space ?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767362642
**[Test build #134478 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134478/testReport)** for PR 31323 at commit [`8dfc2b4`](https://github.com/apache/spark/commit/8dfc2b46455c7397e5e073abe788957fa9193cbb).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766817049
**[Test build #134445 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134445/testReport)** for PR 31323 at commit [`b6bd1f1`](https://github.com/apache/spark/commit/b6bd1f19ede73e2b8038811483a1c40fe6e2abc5).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564281472
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala
##########
@@ -36,8 +37,9 @@ case class QueryExecutionTestRecord(
class QueryExecutionSuite extends SharedSparkSession {
import testImplicits._
- def checkDumpedPlans(path: String, expected: Int): Unit = {
- assert(Source.fromFile(path).getLines.toList
+ def checkDumpedPlans(path: String, expected: Int): Unit = Utils.tryWithResource(
+ Source.fromFile(path)) { source =>
Review comment:
OK ~ thanks ~
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767902088
Thank you, @LuciferYang and @HyukjinKwon and @jaceklaskowski !
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767449225
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134483/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r563757850
##########
File path: examples/src/main/scala/org/apache/spark/examples/DFSReadWriteTest.scala
##########
@@ -46,7 +47,10 @@ object DFSReadWriteTest {
private val NPARAMS = 2
private def readFile(filename: String): List[String] = {
- val lineIter: Iterator[String] = fromFile(filename).getLines()
+ val lineIter: Iterator[String] =
Review comment:
done
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564235162
##########
File path: examples/src/main/scala/org/apache/spark/examples/DFSReadWriteTest.scala
##########
@@ -46,9 +47,7 @@ object DFSReadWriteTest {
private val NPARAMS = 2
private def readFile(filename: String): List[String] = {
- val lineIter: Iterator[String] = fromFile(filename).getLines()
- val lineList: List[String] = lineIter.toList
- lineList
+ Utils.tryWithResource(fromFile(filename))(_.getLines()).toList
Review comment:
done ~
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala
##########
@@ -99,17 +102,19 @@ class QueryExecutionSuite extends SharedSparkSession {
val path = dir.getCanonicalPath + "/plans.txt"
val df = spark.range(0, 10)
df.queryExecution.debug.toFile(path, explainMode = Option("formatted"))
- assert(Source.fromFile(path).getLines.toList
- .takeWhile(_ != "== Whole Stage Codegen ==").map(_.replaceAll("#\\d+", "#x")) == List(
- "== Physical Plan ==",
- s"* Range (1)",
- "",
- "",
- s"(1) Range [codegen id : 1]",
- "Output [1]: [id#xL]",
- s"Arguments: Range (0, 10, step=1, splits=Some(2))",
- "",
- ""))
+ Utils.tryWithResource(Source.fromFile(path)) { source =>
Review comment:
done ~
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767334487
**[Test build #134483 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134483/testReport)** for PR 31323 at commit [`245d844`](https://github.com/apache/spark/commit/245d8446e831043a140b8c16a31a9710328a67a7).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766865833
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39033/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766817071
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134445/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767324706
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39064/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564176054
##########
File path: core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
##########
@@ -327,22 +328,25 @@ class MasterSuite extends SparkFunSuite
val masterUrl = s"http://localhost:${localCluster.masterWebUIPort}"
try {
eventually(timeout(5.seconds), interval(100.milliseconds)) {
- val json = Source.fromURL(s"$masterUrl/json").getLines().mkString("\n")
+ val json = Utils
+ .tryWithResource(Source.fromURL(s"$masterUrl/json"))(_.getLines().mkString("\n"))
val JArray(workers) = (parse(json) \ "workers")
workers.size should be (2)
workers.foreach { workerSummaryJson =>
val JString(workerWebUi) = workerSummaryJson \ "webuiaddress"
- val workerResponse = parse(Source.fromURL(s"${workerWebUi}/json")
- .getLines().mkString("\n"))
+ val workerResponse = parse(Utils
+ .tryWithResource(Source.fromURL(s"$workerWebUi/json"))(_.getLines().mkString("\n")))
Review comment:
The original logic is this, but I can try it.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766852063
**[Test build #134449 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134449/testReport)** for PR 31323 at commit [`4a51d20`](https://github.com/apache/spark/commit/4a51d204f8e22c5a8c2a133a06540b9f13abb63c).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767450249
> Merged to master and branch-3.1.
@LuciferYang, do you mind opening a PR for branch-3.0 and branch-2.4 too?
OK, will do it later ~
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564216000
##########
File path: core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
##########
@@ -327,22 +328,25 @@ class MasterSuite extends SparkFunSuite
val masterUrl = s"http://localhost:${localCluster.masterWebUIPort}"
try {
eventually(timeout(5.seconds), interval(100.milliseconds)) {
- val json = Source.fromURL(s"$masterUrl/json").getLines().mkString("\n")
+ val json = Utils
+ .tryWithResource(Source.fromURL(s"$masterUrl/json"))(_.mkString)
val JArray(workers) = (parse(json) \ "workers")
workers.size should be (2)
workers.foreach { workerSummaryJson =>
val JString(workerWebUi) = workerSummaryJson \ "webuiaddress"
- val workerResponse = parse(Source.fromURL(s"${workerWebUi}/json")
- .getLines().mkString("\n"))
+ val workerResponse = parse(Utils
+ .tryWithResource(Source.fromURL(s"$workerWebUi/json"))(_.mkString))
Review comment:
Let's don't change the behaviour, and keep `"\n"`.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766933120
**[Test build #134449 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134449/testReport)** for PR 31323 at commit [`4a51d20`](https://github.com/apache/spark/commit/4a51d204f8e22c5a8c2a133a06540b9f13abb63c).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766844411
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39033/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766817071
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767422792
**[Test build #134483 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134483/testReport)** for PR 31323 at commit [`245d844`](https://github.com/apache/spark/commit/245d8446e831043a140b8c16a31a9710328a67a7).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766815500
**[Test build #134445 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134445/testReport)** for PR 31323 at commit [`b6bd1f1`](https://github.com/apache/spark/commit/b6bd1f19ede73e2b8038811483a1c40fe6e2abc5).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon closed pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #31323:
URL: https://github.com/apache/spark/pull/31323
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r563758053
##########
File path: core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
##########
@@ -327,22 +328,31 @@ class MasterSuite extends SparkFunSuite
val masterUrl = s"http://localhost:${localCluster.masterWebUIPort}"
try {
eventually(timeout(5.seconds), interval(100.milliseconds)) {
- val json = Source.fromURL(s"$masterUrl/json").getLines().mkString("\n")
+ val json = Utils.tryWithResource(
+ Source.fromURL(s"$masterUrl/json")) { source =>
+ source.getLines().mkString("\n")
Review comment:
thx ~ done
##########
File path: core/src/test/scala/org/apache/spark/deploy/LogUrlsStandaloneSuite.scala
##########
@@ -43,7 +43,9 @@ class LogUrlsStandaloneSuite extends SparkFunSuite with LocalSparkContext {
assert(info.logUrlMap.nonEmpty)
// Browse to each URL to check that it's valid
info.logUrlMap.foreach { case (logType, logUrl) =>
- val html = Source.fromURL(logUrl).mkString
+ val html = Utils.tryWithResource(Source.fromURL(logUrl)) { source =>
+ source.mkString
Review comment:
done
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767373814
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39069/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766817071
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134445/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767439143
Merged to master and branch-3.1.
@LuciferYang, do you mind opening a PR for branch-3.0 and branch-2.4 too?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767383029
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767276672
cc @dongjoon-hyun
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564217717
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala
##########
@@ -99,17 +102,19 @@ class QueryExecutionSuite extends SharedSparkSession {
val path = dir.getCanonicalPath + "/plans.txt"
val df = spark.range(0, 10)
df.queryExecution.debug.toFile(path, explainMode = Option("formatted"))
- assert(Source.fromFile(path).getLines.toList
- .takeWhile(_ != "== Whole Stage Codegen ==").map(_.replaceAll("#\\d+", "#x")) == List(
- "== Physical Plan ==",
- s"* Range (1)",
- "",
- "",
- s"(1) Range [codegen id : 1]",
- "Output [1]: [id#xL]",
- s"Arguments: Range (0, 10, step=1, splits=Some(2))",
- "",
- ""))
+ Utils.tryWithResource(Source.fromFile(path)) { source =>
Review comment:
Can you fix it as:
```scala
val lines = Utils.tryWithResource(Source.fromFile(path))(_.getLines().toList)
assert(lines
.takeWhile(_ != "== Whole Stage Codegen ==").map(_.replaceAll("#\\d+", "#x")) == List(
...
```
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767276672
cc @dongjoon-hyun
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766912121
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39035/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766852063
**[Test build #134449 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134449/testReport)** for PR 31323 at commit [`4a51d20`](https://github.com/apache/spark/commit/4a51d204f8e22c5a8c2a133a06540b9f13abb63c).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767449225
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134483/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564232360
##########
File path: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaTestUtils.scala
##########
@@ -167,29 +167,31 @@ class KafkaTestUtils(
* In this method we rewrite krb5.conf to make kdc and client use the same enctypes
*/
private def rewriteKrb5Conf(): Unit = {
- val krb5Conf = Source.fromFile(kdc.getKrb5conf, "UTF-8").getLines()
- var rewritten = false
- val addedConfig =
- addedKrb5Config("default_tkt_enctypes", "aes128-cts-hmac-sha1-96") +
- addedKrb5Config("default_tgs_enctypes", "aes128-cts-hmac-sha1-96")
- val rewriteKrb5Conf = krb5Conf.map(s =>
- if (s.contains("libdefaults")) {
- rewritten = true
- s + addedConfig
+ Utils.tryWithResource(Source.fromFile(kdc.getKrb5conf, "UTF-8")) { source =>
Review comment:
> sounds better it should be iterated all anyway.
OK ~ use this way
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766934139
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766909087
**[Test build #134447 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134447/testReport)** for PR 31323 at commit [`bc30422`](https://github.com/apache/spark/commit/bc30422e0ac8d0c4ec6a46ae77a6fcfd038e9dc0).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766888964
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39035/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564217279
##########
File path: examples/src/main/scala/org/apache/spark/examples/DFSReadWriteTest.scala
##########
@@ -46,9 +47,7 @@ object DFSReadWriteTest {
private val NPARAMS = 2
private def readFile(filename: String): List[String] = {
- val lineIter: Iterator[String] = fromFile(filename).getLines()
- val lineList: List[String] = lineIter.toList
- lineList
+ Utils.tryWithResource(fromFile(filename))(_.getLines()).toList
Review comment:
I think you should call `getLines().toList` right? because `getLines()` is lazy and it won't consume before you call `toList`.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766815500
**[Test build #134445 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134445/testReport)** for PR 31323 at commit [`b6bd1f1`](https://github.com/apache/spark/commit/b6bd1f19ede73e2b8038811483a1c40fe6e2abc5).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766849682
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39031/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564281472
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala
##########
@@ -36,8 +37,9 @@ case class QueryExecutionTestRecord(
class QueryExecutionSuite extends SharedSparkSession {
import testImplicits._
- def checkDumpedPlans(path: String, expected: Int): Unit = {
- assert(Source.fromFile(path).getLines.toList
+ def checkDumpedPlans(path: String, expected: Int): Unit = Utils.tryWithResource(
+ Source.fromFile(path)) { source =>
Review comment:
OK ~
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766826242
**[Test build #134447 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134447/testReport)** for PR 31323 at commit [`bc30422`](https://github.com/apache/spark/commit/bc30422e0ac8d0c4ec6a46ae77a6fcfd038e9dc0).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564230679
##########
File path: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaTestUtils.scala
##########
@@ -167,29 +167,31 @@ class KafkaTestUtils(
* In this method we rewrite krb5.conf to make kdc and client use the same enctypes
*/
private def rewriteKrb5Conf(): Unit = {
Review comment:
replace with `Utils.tryWithResource(Source.fromFile(kdc.getKrb5conf, "UTF-8"))(_.getLines().toList)`, only 1 line changed
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564209367
##########
File path: core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
##########
@@ -327,22 +328,25 @@ class MasterSuite extends SparkFunSuite
val masterUrl = s"http://localhost:${localCluster.masterWebUIPort}"
try {
eventually(timeout(5.seconds), interval(100.milliseconds)) {
- val json = Source.fromURL(s"$masterUrl/json").getLines().mkString("\n")
+ val json = Utils
+ .tryWithResource(Source.fromURL(s"$masterUrl/json"))(_.getLines().mkString("\n"))
val JArray(workers) = (parse(json) \ "workers")
workers.size should be (2)
workers.foreach { workerSummaryJson =>
val JString(workerWebUi) = workerSummaryJson \ "webuiaddress"
- val workerResponse = parse(Source.fromURL(s"${workerWebUi}/json")
- .getLines().mkString("\n"))
+ val workerResponse = parse(Utils
+ .tryWithResource(Source.fromURL(s"$workerWebUi/json"))(_.getLines().mkString("\n")))
Review comment:
Seems can use `.mkString` instead of `_.getLines().mkString("\n")`
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767310462
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39064/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767287966
**[Test build #134478 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134478/testReport)** for PR 31323 at commit [`8dfc2b4`](https://github.com/apache/spark/commit/8dfc2b46455c7397e5e073abe788957fa9193cbb).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-767301174
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39064/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564236924
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalogVersionsSuite.scala
##########
@@ -242,8 +242,9 @@ object PROCESS_TABLES extends QueryTest with SQLTestUtils {
// Tests the latest version of every release line.
val testingVersions: Seq[String] = {
import scala.io.Source
- val versions: Seq[String] = try {
- Source.fromURL(s"${releaseMirror}/spark").mkString
+ val versions: Seq[String] = try Utils.tryWithResource(
+ Source.fromURL(s"$releaseMirror/spark")) { source =>
Review comment:
@HyukjinKwon line 246 should have 4 space or 2 space ?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766826242
**[Test build #134447 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134447/testReport)** for PR 31323 at commit [`bc30422`](https://github.com/apache/spark/commit/bc30422e0ac8d0c4ec6a46ae77a6fcfd038e9dc0).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564236924
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalogVersionsSuite.scala
##########
@@ -242,8 +242,9 @@ object PROCESS_TABLES extends QueryTest with SQLTestUtils {
// Tests the latest version of every release line.
val testingVersions: Seq[String] = {
import scala.io.Source
- val versions: Seq[String] = try {
- Source.fromURL(s"${releaseMirror}/spark").mkString
+ val versions: Seq[String] = try Utils.tryWithResource(
+ Source.fromURL(s"$releaseMirror/spark")) { source =>
Review comment:
@HyukjinKwon 4 space or 2 space ?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564215240
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalogVersionsSuite.scala
##########
@@ -243,12 +243,14 @@ object PROCESS_TABLES extends QueryTest with SQLTestUtils {
val testingVersions: Seq[String] = {
import scala.io.Source
val versions: Seq[String] = try {
Review comment:
this too:
```scala
val versions: Seq[String] = try Utils.tryWithResource(
Source.fromURL(s"$releaseMirror/spark")) { source =>
// blah blah
} catch {
// blah blah
}
```
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31323:
URL: https://github.com/apache/spark/pull/31323#issuecomment-766817071
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564273754
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala
##########
@@ -36,8 +37,9 @@ case class QueryExecutionTestRecord(
class QueryExecutionSuite extends SharedSparkSession {
import testImplicits._
- def checkDumpedPlans(path: String, expected: Int): Unit = {
- assert(Source.fromFile(path).getLines.toList
+ def checkDumpedPlans(path: String, expected: Int): Unit = Utils.tryWithResource(
+ Source.fromFile(path)) { source =>
Review comment:
I would use 4 spaces here but it's fine. I'll leave it to you.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jaceklaskowski commented on a change in pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
jaceklaskowski commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r563724184
##########
File path: core/src/test/scala/org/apache/spark/deploy/LogUrlsStandaloneSuite.scala
##########
@@ -43,7 +43,9 @@ class LogUrlsStandaloneSuite extends SparkFunSuite with LocalSparkContext {
assert(info.logUrlMap.nonEmpty)
// Browse to each URL to check that it's valid
info.logUrlMap.foreach { case (logType, logUrl) =>
- val html = Source.fromURL(logUrl).mkString
+ val html = Utils.tryWithResource(Source.fromURL(logUrl)) { source =>
+ source.mkString
Review comment:
nit: `_.mkString`
##########
File path: core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
##########
@@ -327,22 +328,31 @@ class MasterSuite extends SparkFunSuite
val masterUrl = s"http://localhost:${localCluster.masterWebUIPort}"
try {
eventually(timeout(5.seconds), interval(100.milliseconds)) {
- val json = Source.fromURL(s"$masterUrl/json").getLines().mkString("\n")
+ val json = Utils.tryWithResource(
+ Source.fromURL(s"$masterUrl/json")) { source =>
+ source.getLines().mkString("\n")
Review comment:
nit: Indent (and perhaps `_.getLines...` too?)
##########
File path: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaSourceOffsetSuite.scala
##########
@@ -99,7 +100,9 @@ class KafkaSourceOffsetSuite extends OffsetSuite with SharedSparkSession {
private def readFromResource(file: String): SerializedOffset = {
import scala.io.Source
val input = getClass.getResource(s"/$file").toURI
- val str = Source.fromFile(input).mkString
- SerializedOffset(str)
+ Utils.tryWithResource(Source.fromFile(input)) { source =>
+ val str = source.mkString
Review comment:
Any reason for this `str` value?
##########
File path: core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
##########
@@ -327,22 +328,31 @@ class MasterSuite extends SparkFunSuite
val masterUrl = s"http://localhost:${localCluster.masterWebUIPort}"
try {
eventually(timeout(5.seconds), interval(100.milliseconds)) {
- val json = Source.fromURL(s"$masterUrl/json").getLines().mkString("\n")
+ val json = Utils.tryWithResource(
+ Source.fromURL(s"$masterUrl/json")) { source =>
+ source.getLines().mkString("\n")
+ }
val JArray(workers) = (parse(json) \ "workers")
workers.size should be (2)
workers.foreach { workerSummaryJson =>
val JString(workerWebUi) = workerSummaryJson \ "webuiaddress"
- val workerResponse = parse(Source.fromURL(s"${workerWebUi}/json")
- .getLines().mkString("\n"))
+ val workerResponse = Utils.tryWithResource(
+ Source.fromURL(s"$workerWebUi/json")) { source =>
+ parse(source.getLines().mkString("\n"))
Review comment:
nit: Indent
##########
File path: examples/src/main/scala/org/apache/spark/examples/DFSReadWriteTest.scala
##########
@@ -46,7 +47,10 @@ object DFSReadWriteTest {
private val NPARAMS = 2
private def readFile(filename: String): List[String] = {
- val lineIter: Iterator[String] = fromFile(filename).getLines()
+ val lineIter: Iterator[String] =
Review comment:
While we're at it...
```
Utils.tryWithResource(fromFile(filename))(_.getLines).toList
```
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564216942
##########
File path: core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
##########
@@ -327,22 +328,25 @@ class MasterSuite extends SparkFunSuite
val masterUrl = s"http://localhost:${localCluster.masterWebUIPort}"
try {
eventually(timeout(5.seconds), interval(100.milliseconds)) {
- val json = Source.fromURL(s"$masterUrl/json").getLines().mkString("\n")
+ val json = Utils
+ .tryWithResource(Source.fromURL(s"$masterUrl/json"))(_.mkString)
Review comment:
I know you changed per the review comment above but let's keep the contents. At least it wouldn't keep the original files as are anymore, and it's strictly a different issue from what you're fixing here.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][TEST] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r563757768
##########
File path: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaSourceOffsetSuite.scala
##########
@@ -99,7 +100,9 @@ class KafkaSourceOffsetSuite extends OffsetSuite with SharedSparkSession {
private def readFromResource(file: String): SerializedOffset = {
import scala.io.Source
val input = getClass.getResource(s"/$file").toURI
- val str = Source.fromFile(input).mkString
- SerializedOffset(str)
+ Utils.tryWithResource(Source.fromFile(input)) { source =>
+ val str = source.mkString
Review comment:
removed
##########
File path: examples/src/main/scala/org/apache/spark/examples/DFSReadWriteTest.scala
##########
@@ -46,7 +47,10 @@ object DFSReadWriteTest {
private val NPARAMS = 2
private def readFile(filename: String): List[String] = {
- val lineIter: Iterator[String] = fromFile(filename).getLines()
+ val lineIter: Iterator[String] =
Review comment:
done
##########
File path: core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
##########
@@ -327,22 +328,31 @@ class MasterSuite extends SparkFunSuite
val masterUrl = s"http://localhost:${localCluster.masterWebUIPort}"
try {
eventually(timeout(5.seconds), interval(100.milliseconds)) {
- val json = Source.fromURL(s"$masterUrl/json").getLines().mkString("\n")
+ val json = Utils.tryWithResource(
+ Source.fromURL(s"$masterUrl/json")) { source =>
+ source.getLines().mkString("\n")
Review comment:
thx ~ done
##########
File path: core/src/test/scala/org/apache/spark/deploy/LogUrlsStandaloneSuite.scala
##########
@@ -43,7 +43,9 @@ class LogUrlsStandaloneSuite extends SparkFunSuite with LocalSparkContext {
assert(info.logUrlMap.nonEmpty)
// Browse to each URL to check that it's valid
info.logUrlMap.foreach { case (logType, logUrl) =>
- val html = Source.fromURL(logUrl).mkString
+ val html = Utils.tryWithResource(Source.fromURL(logUrl)) { source =>
+ source.mkString
Review comment:
done
##########
File path: core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
##########
@@ -327,22 +328,25 @@ class MasterSuite extends SparkFunSuite
val masterUrl = s"http://localhost:${localCluster.masterWebUIPort}"
try {
eventually(timeout(5.seconds), interval(100.milliseconds)) {
- val json = Source.fromURL(s"$masterUrl/json").getLines().mkString("\n")
+ val json = Utils
+ .tryWithResource(Source.fromURL(s"$masterUrl/json"))(_.getLines().mkString("\n"))
val JArray(workers) = (parse(json) \ "workers")
workers.size should be (2)
workers.foreach { workerSummaryJson =>
val JString(workerWebUi) = workerSummaryJson \ "webuiaddress"
- val workerResponse = parse(Source.fromURL(s"${workerWebUi}/json")
- .getLines().mkString("\n"))
+ val workerResponse = parse(Utils
+ .tryWithResource(Source.fromURL(s"$workerWebUi/json"))(_.getLines().mkString("\n")))
Review comment:
The original logic is this, but I can try it.
##########
File path: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaTestUtils.scala
##########
@@ -167,29 +167,31 @@ class KafkaTestUtils(
* In this method we rewrite krb5.conf to make kdc and client use the same enctypes
*/
private def rewriteKrb5Conf(): Unit = {
- val krb5Conf = Source.fromFile(kdc.getKrb5conf, "UTF-8").getLines()
- var rewritten = false
- val addedConfig =
- addedKrb5Config("default_tkt_enctypes", "aes128-cts-hmac-sha1-96") +
- addedKrb5Config("default_tgs_enctypes", "aes128-cts-hmac-sha1-96")
- val rewriteKrb5Conf = krb5Conf.map(s =>
- if (s.contains("libdefaults")) {
- rewritten = true
- s + addedConfig
+ Utils.tryWithResource(Source.fromFile(kdc.getKrb5conf, "UTF-8")) { source =>
Review comment:
```
sbt.ForkMain$ForkError: java.io.IOException: Stream Closed
at java.io.FileInputStream.readBytes(Native Method)
at java.io.FileInputStream.read(FileInputStream.java:255)
at sun.nio.cs.StreamDecoder.readBytes(StreamDecoder.java:284)
at sun.nio.cs.StreamDecoder.implRead(StreamDecoder.java:326)
at sun.nio.cs.StreamDecoder.read(StreamDecoder.java:178)
at java.io.InputStreamReader.read(InputStreamReader.java:184)
at java.io.BufferedReader.fill(BufferedReader.java:161)
at java.io.BufferedReader.readLine(BufferedReader.java:324)
at java.io.BufferedReader.readLine(BufferedReader.java:389)
at scala.io.BufferedSource$BufferedLineIterator.hasNext(BufferedSource.scala:74)
at scala.collection.Iterator$$anon$10.hasNext(Iterator.scala:460)
at scala.collection.Iterator$$anon$12.hasNext(Iterator.scala:513)
at scala.collection.Iterator.foreach(Iterator.scala:943)
at scala.collection.Iterator.foreach$(Iterator.scala:943)
at scala.collection.AbstractIterator.foreach(Iterator.scala:1431)
at scala.collection.TraversableOnce.addString(TraversableOnce.scala:424)
at scala.collection.TraversableOnce.addString$(TraversableOnce.scala:407)
at scala.collection.AbstractIterator.addString(Iterator.scala:1431)
at scala.collection.TraversableOnce.mkString(TraversableOnce.scala:377)
at scala.collection.TraversableOnce.mkString$(TraversableOnce.scala:376)
at scala.collection.AbstractIterator.mkString(Iterator.scala:1431)
at scala.collection.TraversableOnce.mkString(TraversableOnce.scala:379)
at scala.collection.TraversableOnce.mkString$(TraversableOnce.scala:379)
at scala.collection.AbstractIterator.mkString(Iterator.scala:1431)
at org.apache.spark.sql.kafka010.KafkaTestUtils.rewriteKrb5Conf(KafkaTestUtils.scala:182)
at org.apache.spark.sql.kafka010.KafkaTestUtils.setUpMiniKdc(KafkaTestUtils.scala:162)
at org.apache.spark.sql.kafka010.KafkaTestUtils.setup(KafkaTestUtils.scala:304)
at org.apache.spark.sql.kafka010.KafkaDelegationTokenSuite.beforeAll(KafkaDelegationTokenSuite.scala:49)
```
`Utils.tryWithResource(Source.fromFile(kdc.getKrb5conf, "UTF-8")) (_.getLines())` seems return a lazy iter, so there expansion` Utils.tryWithResource` scope to avoid `java.io.IOException: Stream Closed`.
Or we need to materialize `Utils.tryWithResource(Source.fromFile(kdc.getKrb5conf, "UTF-8")) (_.getLines())` like
`Utils.tryWithResource(Source.fromFile(kdc.getKrb5conf, "UTF-8")) (_.getLines().toList)`
Which one would you choose @dongjoon-hyun
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564215658
##########
File path: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaTestUtils.scala
##########
@@ -167,29 +167,31 @@ class KafkaTestUtils(
* In this method we rewrite krb5.conf to make kdc and client use the same enctypes
*/
private def rewriteKrb5Conf(): Unit = {
- val krb5Conf = Source.fromFile(kdc.getKrb5conf, "UTF-8").getLines()
- var rewritten = false
- val addedConfig =
- addedKrb5Config("default_tkt_enctypes", "aes128-cts-hmac-sha1-96") +
- addedKrb5Config("default_tgs_enctypes", "aes128-cts-hmac-sha1-96")
- val rewriteKrb5Conf = krb5Conf.map(s =>
- if (s.contains("libdefaults")) {
- rewritten = true
- s + addedConfig
+ Utils.tryWithResource(Source.fromFile(kdc.getKrb5conf, "UTF-8")) { source =>
Review comment:
```
Utils.tryWithResource(Source.fromFile(kdc.getKrb5conf, "UTF-8")) (_.getLines().toList)
```
sounds better it should be iterated all anyway.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a change in pull request #31323: [SPARK-34224][CORE][SQL][SS][DSTREAM][YARN][TEST][EXAMPLES] Ensure all resource opened by `Source.fromXXX` are closed
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #31323:
URL: https://github.com/apache/spark/pull/31323#discussion_r564223913
##########
File path: core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala
##########
@@ -327,22 +328,25 @@ class MasterSuite extends SparkFunSuite
val masterUrl = s"http://localhost:${localCluster.masterWebUIPort}"
try {
eventually(timeout(5.seconds), interval(100.milliseconds)) {
- val json = Source.fromURL(s"$masterUrl/json").getLines().mkString("\n")
+ val json = Utils
+ .tryWithResource(Source.fromURL(s"$masterUrl/json"))(_.mkString)
Review comment:
ok
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org