Re: [PR] [SPARK-45701][SPARK-45684][SPARK-45692][CORE][SQL][SS][ML][K8S] Clean up the deprecated API usage related to `mutable.SetOps/c.SeqOps/Iterator/Iterable/IterableOps` [spark]
LuciferYang commented on code in PR #43575:
URL: https://github.com/apache/spark/pull/43575#discussion_r1414253765
##
sql/core/src/test/scala/org/apache/spark/sql/streaming/StreamSuite.scala:
##
@@ -174,7 +174,7 @@ class StreamSuite extends StreamTest {
try {
query.processAllAvailable()
val outputDf = spark.read.parquet(outputDir.getAbsolutePath).as[Long]
- checkDatasetUnorderly[Long](outputDf, (0L to 10L).union((0L to
10L)).toArray: _*)
+ checkDatasetUnorderly[Long](outputDf, (0L to 10L).concat(0L to 10L):
_*)
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.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-45701][SPARK-45684][SPARK-45692][CORE][SQL][SS][ML][K8S] Clean up the deprecated API usage related to `mutable.SetOps/c.SeqOps/Iterator/Iterable/IterableOps` [spark]
cloud-fan commented on code in PR #43575:
URL: https://github.com/apache/spark/pull/43575#discussion_r1414252458
##
sql/core/src/test/scala/org/apache/spark/sql/streaming/StreamSuite.scala:
##
@@ -174,7 +174,7 @@ class StreamSuite extends StreamTest {
try {
query.processAllAvailable()
val outputDf = spark.read.parquet(outputDir.getAbsolutePath).as[Long]
- checkDatasetUnorderly[Long](outputDf, (0L to 10L).union((0L to
10L)).toArray: _*)
+ checkDatasetUnorderly[Long](outputDf, (0L to 10L).concat(0L to 10L):
_*)
Review Comment:
This is the only one I found in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-45701][SPARK-45684][SPARK-45692][CORE][SQL][SS][ML][K8S] Clean up the deprecated API usage related to `mutable.SetOps/c.SeqOps/Iterator/Iterable/IterableOps` [spark]
LuciferYang commented on code in PR #43575:
URL: https://github.com/apache/spark/pull/43575#discussion_r1414176075
##
sql/core/src/test/scala/org/apache/spark/sql/streaming/StreamSuite.scala:
##
@@ -174,7 +174,7 @@ class StreamSuite extends StreamTest {
try {
query.processAllAvailable()
val outputDf = spark.read.parquet(outputDir.getAbsolutePath).as[Long]
- checkDatasetUnorderly[Long](outputDf, (0L to 10L).union((0L to
10L)).toArray: _*)
+ checkDatasetUnorderly[Long](outputDf, (0L to 10L).concat(0L to 10L):
_*)
Review Comment:
Are there any other issues that need to be fixed in this pr? I will submit a
PR to fix them all together 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.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-45701][SPARK-45684][SPARK-45692][CORE][SQL][SS][ML][K8S] Clean up the deprecated API usage related to `mutable.SetOps/c.SeqOps/Iterator/Iterable/IterableOps` [spark]
LuciferYang commented on code in PR #43575:
URL: https://github.com/apache/spark/pull/43575#discussion_r1414168092
##
sql/core/src/test/scala/org/apache/spark/sql/streaming/StreamSuite.scala:
##
@@ -174,7 +174,7 @@ class StreamSuite extends StreamTest {
try {
query.processAllAvailable()
val outputDf = spark.read.parquet(outputDir.getAbsolutePath).as[Long]
- checkDatasetUnorderly[Long](outputDf, (0L to 10L).union((0L to
10L)).toArray: _*)
+ checkDatasetUnorderly[Long](outputDf, (0L to 10L).concat(0L to 10L):
_*)
Review Comment:
Yes, `++` is calling `concat`, but `++` should be better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-45701][SPARK-45684][SPARK-45692][CORE][SQL][SS][ML][K8S] Clean up the deprecated API usage related to `mutable.SetOps/c.SeqOps/Iterator/Iterable/IterableOps` [spark]
LuciferYang commented on code in PR #43575:
URL: https://github.com/apache/spark/pull/43575#discussion_r1414168092
##
sql/core/src/test/scala/org/apache/spark/sql/streaming/StreamSuite.scala:
##
@@ -174,7 +174,7 @@ class StreamSuite extends StreamTest {
try {
query.processAllAvailable()
val outputDf = spark.read.parquet(outputDir.getAbsolutePath).as[Long]
- checkDatasetUnorderly[Long](outputDf, (0L to 10L).union((0L to
10L)).toArray: _*)
+ checkDatasetUnorderly[Long](outputDf, (0L to 10L).concat(0L to 10L):
_*)
Review Comment:
Yes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-45701][SPARK-45684][SPARK-45692][CORE][SQL][SS][ML][K8S] Clean up the deprecated API usage related to `mutable.SetOps/c.SeqOps/Iterator/Iterable/IterableOps` [spark]
cloud-fan commented on code in PR #43575:
URL: https://github.com/apache/spark/pull/43575#discussion_r1414163363
##
sql/core/src/test/scala/org/apache/spark/sql/streaming/StreamSuite.scala:
##
@@ -174,7 +174,7 @@ class StreamSuite extends StreamTest {
try {
query.processAllAvailable()
val outputDf = spark.read.parquet(outputDir.getAbsolutePath).as[Long]
- checkDatasetUnorderly[Long](outputDf, (0L to 10L).union((0L to
10L)).toArray: _*)
+ checkDatasetUnorderly[Long](outputDf, (0L to 10L).concat(0L to 10L):
_*)
Review Comment:
is it just `(0L to 10L) ++ ((0L to 10L))`?
##
sql/core/src/test/scala/org/apache/spark/sql/streaming/StreamSuite.scala:
##
@@ -174,7 +174,7 @@ class StreamSuite extends StreamTest {
try {
query.processAllAvailable()
val outputDf = spark.read.parquet(outputDir.getAbsolutePath).as[Long]
- checkDatasetUnorderly[Long](outputDf, (0L to 10L).union((0L to
10L)).toArray: _*)
+ checkDatasetUnorderly[Long](outputDf, (0L to 10L).concat(0L to 10L):
_*)
Review Comment:
is it just `(0L to 10L) ++ (0L to 10L)`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-45701][SPARK-45684][SPARK-45692][CORE][SQL][SS][ML][K8S] Clean up the deprecated API usage related to `mutable.SetOps/c.SeqOps/Iterator/Iterable/IterableOps` [spark]
LuciferYang commented on PR #43575: URL: https://github.com/apache/spark/pull/43575#issuecomment-1786582233 Merged into master for Spark 4.0. Thanks @HyukjinKwon @dongjoon-hyun @beliefer -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-45701][SPARK-45684][SPARK-45692][CORE][SQL][SS][ML][K8S] Clean up the deprecated API usage related to `mutable.SetOps/c.SeqOps/Iterator/Iterable/IterableOps` [spark]
LuciferYang closed pull request #43575: [SPARK-45701][SPARK-45684][SPARK-45692][CORE][SQL][SS][ML][K8S] Clean up the deprecated API usage related to `mutable.SetOps/c.SeqOps/Iterator/Iterable/IterableOps` URL: https://github.com/apache/spark/pull/43575 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-45701][SPARK-45684][SPARK-45692][CORE][SQL][SS][ML][K8S] Clean up the deprecated API usage related to `mutable.SetOps/c.SeqOps/Iterator/Iterable/IterableOps` [spark]
dongjoon-hyun commented on code in PR #43575: URL: https://github.com/apache/spark/pull/43575#discussion_r1375697961 ## connector/connect/server/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala: ## @@ -3183,9 +3184,9 @@ class SparkConnectPlanner( case StreamingQueryManagerCommand.CommandCase.ACTIVE => val active_queries = session.streams.active respBuilder.getActiveBuilder.addAllActiveQueries( - active_queries -.map(query => buildStreamingQueryInstance(query)) -.toIterable + immutable.ArraySeq +.unsafeWrapArray(active_queries + .map(query => buildStreamingQueryInstance(query))) Review Comment: Oh, got it. Thank you, @LuciferYang . -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-45701][SPARK-45684][SPARK-45692][CORE][SQL][SS][ML][K8S] Clean up the deprecated API usage related to `mutable.SetOps/c.SeqOps/Iterator/Iterable/IterableOps` [spark]
LuciferYang commented on code in PR #43575: URL: https://github.com/apache/spark/pull/43575#discussion_r1375694433 ## connector/connect/server/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala: ## @@ -3183,9 +3184,9 @@ class SparkConnectPlanner( case StreamingQueryManagerCommand.CommandCase.ACTIVE => val active_queries = session.streams.active respBuilder.getActiveBuilder.addAllActiveQueries( - active_queries -.map(query => buildStreamingQueryInstance(query)) -.toIterable + immutable.ArraySeq +.unsafeWrapArray(active_queries + .map(query => buildStreamingQueryInstance(query))) Review Comment: The code in the connect module must use scalafmt... the result of this scalafmt is mandatory, otherwise, the lint will fail the check :( -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-45701][SPARK-45684][SPARK-45692][CORE][SQL][SS][ML][K8S] Clean up the deprecated API usage related to `mutable.SetOps/c.SeqOps/Iterator/Iterable/IterableOps` [spark]
dongjoon-hyun commented on code in PR #43575: URL: https://github.com/apache/spark/pull/43575#discussion_r1375693346 ## connector/connect/server/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala: ## @@ -3183,9 +3184,9 @@ class SparkConnectPlanner( case StreamingQueryManagerCommand.CommandCase.ACTIVE => val active_queries = session.streams.active respBuilder.getActiveBuilder.addAllActiveQueries( - active_queries -.map(query => buildStreamingQueryInstance(query)) -.toIterable + immutable.ArraySeq +.unsafeWrapArray(active_queries + .map(query => buildStreamingQueryInstance(query))) Review Comment: nit. In this case, one liner could be better and consistent with other parts. ``` .unsafeWrapArray(active_queries.map(query => buildStreamingQueryInstance(query))) ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-45701][SPARK-45684][SPARK-45692][CORE][SQL][SS][ML][K8S] Clean up the deprecated API usage related to `mutable.SetOps/c.SeqOps/Iterator/Iterable/IterableOps` [spark]
LuciferYang commented on PR #43575: URL: https://github.com/apache/spark/pull/43575#issuecomment-1784464634 I've merged changes related to SPARK-45684 and SPARK-45692 into this one. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
