You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "dupen01 (via GitHub)" <gi...@apache.org> on 2024/08/09 07:59:37 UTC

[PR] Upgrade Scala version from 2.13.8 to 2.13.11 [kyuubi]

dupen01 opened a new pull request, #6603:
URL: https://github.com/apache/kyuubi/pull/6603

   # :mag: Description
   ## Issue References ๐Ÿ”—
   
   To support Java 21, the Scala version needs to be upgraded to 2.13.11 or above.
   
   
   
   
   
   ## Types of changes :bookmark:
   <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
   - [ ] Bugfix (non-breaking change which fixes an issue)
   - [x] New feature (non-breaking change which adds functionality)
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   
   ## Test Plan ๐Ÿงช
   
   #### Behavior Without This Pull Request :coffin:
   
   
   #### Behavior With This Pull Request :tada:
   
   
   #### Related Unit Tests
   
   
   ---
   
   # Checklist ๐Ÿ“
   <!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
   <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
   
   - [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)
   
   **Be nice. Be informative.**
   


-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#discussion_r1713300650


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -80,7 +80,8 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
   hikariConfig.setPoolName("jdbc-metadata-store-pool")
 
   @VisibleForTesting
-  implicit private[kyuubi] val hikariDataSource = new HikariDataSource(hikariConfig)
+  implicit private[kyuubi] val hikariDataSource: HikariDataSource =
+    new HikariDataSource(hikariConfig)

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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#issuecomment-2278104245

   Seems `FinalStageResourceManagerSuite` gets stuck after upgrading Scala 2.13, cc @zhouyifan279 do you have time to take a look? thanks in advance


-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.11 [kyuubi]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#issuecomment-2277385721

   can we use Scala 2.13.14 to align with Spark 4.0.0-preview1?


-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "dupen01 (via GitHub)" <gi...@apache.org>.
dupen01 commented on code in PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#discussion_r1713296988


##########
pom.xml:
##########
@@ -1853,6 +1853,9 @@
                                 <enforceBytecodeVersion>
                                     <maxJdkVersion>${java.version}</maxJdkVersion>
                                     <ignoredScopes>test</ignoredScopes>
+                                    <ignoreClasses>
+                                        <ignoreClass>org.jline.terminal.impl.ffm.*</ignoreClass>
+                                    </ignoreClasses>

Review Comment:
   As @pan3793 mentioned: https://github.com/apache/spark/pull/46288



-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "dupen01 (via GitHub)" <gi...@apache.org>.
dupen01 commented on code in PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#discussion_r1713302899


##########
pom.xml:
##########
@@ -1853,6 +1853,9 @@
                                 <enforceBytecodeVersion>
                                     <maxJdkVersion>${java.version}</maxJdkVersion>
                                     <ignoredScopes>test</ignoredScopes>
+                                    <ignoreClasses>
+                                        <ignoreClass>org.jline.terminal.impl.ffm.*</ignoreClass>
+                                    </ignoreClasses>

Review Comment:
   Sure, I will add the comments.



-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#discussion_r1713299719


##########
pom.xml:
##########
@@ -1853,6 +1853,9 @@
                                 <enforceBytecodeVersion>
                                     <maxJdkVersion>${java.version}</maxJdkVersion>
                                     <ignoredScopes>test</ignoredScopes>
+                                    <ignoreClasses>
+                                        <ignoreClass>org.jline.terminal.impl.ffm.*</ignoreClass>
+                                    </ignoreClasses>

Review Comment:
   OK. But how about bringing the comments as well ? https://github.com/apache/spark/pull/46288/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2985



-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#issuecomment-2277568008

   let's follow https://github.com/apache/spark/pull/46288 to add ignore 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.

To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#discussion_r1713220828


##########
pom.xml:
##########
@@ -241,7 +241,7 @@
         <maven.plugin.jacoco.version>0.8.11</maven.plugin.jacoco.version>
         <maven.plugin.scalastyle.version>1.0.0</maven.plugin.scalastyle.version>
         <maven.plugin.shade.version>3.5.2</maven.plugin.shade.version>
-        <maven.plugin.silencer.version>1.7.13</maven.plugin.silencer.version>
+        <maven.plugin.silencer.version>1.7.17</maven.plugin.silencer.version>

Review Comment:
   Why we have to update the silencer version?



-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#issuecomment-2304122237

   I checked the failure message from `FinalStageResourceManagerSuite` 
   
   ```
   16:21:05.237 ERROR org.apache.spark.network.server.TransportRequestHandler: Error while invoking RpcHandler#receive() on RPC id 9220236832923458123
   java.io.InvalidClassException: scala.concurrent.duration.FiniteDuration; local class incompatible: stream classdesc serialVersionUID = -4594686286536372853, local class serialVersionUID = -6513803676778706429
           at java.base/java.io.ObjectStreamClass.initNonProxy(ObjectStreamClass.java:597)
           ...
           at org.apache.spark.serializer.JavaDeserializationStream.readObject(JavaSerializer.scala:87)
           at org.apache.spark.serializer.JavaSerializerInstance.deserialize(JavaSerializer.scala:123)
           at org.apache.spark.rpc.netty.NettyRpcEnv.$anonfun$deserialize$2(NettyRpcEnv.scala:299)
           at scala.util.DynamicVariable.withValue(DynamicVariable.scala:59)
           at org.apache.spark.rpc.netty.NettyRpcEnv.deserialize(NettyRpcEnv.scala:352)
   ```
   
   the root cause is that it launches a local cluster using SPARK_HOME, thus using a different Scala library with SPARK_HOME may cause serialization issues as JavaSerializer is not used 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.

To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.11 [kyuubi]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#issuecomment-2277389051

   BTW, please rebase your code on latest master branch, there were some issues on CI in my previous commits


-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#discussion_r1711438228


##########
pom.xml:
##########
@@ -1853,6 +1853,10 @@
                                 <enforceBytecodeVersion>
                                     <maxJdkVersion>${java.version}</maxJdkVersion>
                                     <ignoredScopes>test</ignoredScopes>
+                                    <ignoredScopes>provided</ignoredScopes>

Review Comment:
   ```suggestion
   ```



-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#discussion_r1713220453


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -80,7 +80,8 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
   hikariConfig.setPoolName("jdbc-metadata-store-pool")
 
   @VisibleForTesting
-  implicit private[kyuubi] val hikariDataSource = new HikariDataSource(hikariConfig)
+  implicit private[kyuubi] val hikariDataSource: HikariDataSource =
+    new HikariDataSource(hikariConfig)

Review Comment:
   ```suggestion
     implicit private[kyuubi] val hikariDataSource = new HikariDataSource(hikariConfig)
   ```
   Revert unnecessary change.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#discussion_r1713228281


##########
pom.xml:
##########
@@ -241,7 +241,7 @@
         <maven.plugin.jacoco.version>0.8.11</maven.plugin.jacoco.version>
         <maven.plugin.scalastyle.version>1.0.0</maven.plugin.scalastyle.version>
         <maven.plugin.shade.version>3.5.2</maven.plugin.shade.version>
-        <maven.plugin.silencer.version>1.7.13</maven.plugin.silencer.version>
+        <maven.plugin.silencer.version>1.7.17</maven.plugin.silencer.version>

Review Comment:
   Cool. SGTM. Thanks for the explanation.



##########
pom.xml:
##########
@@ -1853,6 +1853,9 @@
                                 <enforceBytecodeVersion>
                                     <maxJdkVersion>${java.version}</maxJdkVersion>
                                     <ignoredScopes>test</ignoredScopes>
+                                    <ignoreClasses>
+                                        <ignoreClass>org.jline.terminal.impl.ffm.*</ignoreClass>
+                                    </ignoreClasses>

Review Comment:
   For what reasons, we have to ignore these classes? Any docs, reference, clarification for 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.

To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#discussion_r1713221448


##########
pom.xml:
##########
@@ -1853,6 +1853,9 @@
                                 <enforceBytecodeVersion>
                                     <maxJdkVersion>${java.version}</maxJdkVersion>
                                     <ignoredScopes>test</ignoredScopes>
+                                    <ignoreClasses>
+                                        <ignoreClass>org.jline.terminal.impl.ffm.*</ignoreClass>
+                                    </ignoreClasses>

Review Comment:
   For what reasons, we have to ignore these classes? Any docs, reference, clarification for this?



-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#issuecomment-2287840452

   ## [Codecov](https://app.codecov.io/gh/apache/kyuubi/pull/6603?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   Attention: Patch coverage is `0%` with `10 lines` in your changes missing coverage. Please review.
   > Project coverage is 0.00%. Comparing base [(`bdd91f4`)](https://app.codecov.io/gh/apache/kyuubi/commit/bdd91f45396ab20b6dfe0a266fb8bfe340d99192?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`e3f4625`)](https://app.codecov.io/gh/apache/kyuubi/commit/e3f46255fb8b60c235657aa1ff2751fbeba6d824?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   > Report is 5 commits behind head on master.
   
   | [Files](https://app.codecov.io/gh/apache/kyuubi/pull/6603?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...apache/kyuubi/session/FileSessionConfAdvisor.scala](https://app.codecov.io/gh/apache/kyuubi/pull/6603?src=pr&el=tree&filepath=kyuubi-server%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fkyuubi%2Fsession%2FFileSessionConfAdvisor.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL0ZpbGVTZXNzaW9uQ29uZkFkdmlzb3Iuc2NhbGE=) | 0.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/6603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://app.codecov.io/gh/apache/kyuubi/pull/6603?src=pr&el=tree&filepath=kyuubi-common%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fkyuubi%2Fconfig%2FKyuubiConf.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/6603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...yuubi/server/metadata/jdbc/JDBCMetadataStore.scala](https://app.codecov.io/gh/apache/kyuubi/pull/6603?src=pr&el=tree&filepath=kyuubi-server%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fkyuubi%2Fserver%2Fmetadata%2Fjdbc%2FJDBCMetadataStore.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbWV0YWRhdGEvamRiYy9KREJDTWV0YWRhdGFTdG9yZS5zY2FsYQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/6603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@          Coverage Diff           @@
   ##           master   #6603   +/-   ##
   ======================================
     Coverage    0.00%   0.00%           
   ======================================
     Files         677     677           
     Lines       41902   41908    +6     
     Branches     5721    5721           
   ======================================
   - Misses      41902   41908    +6     
   ```
   
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/kyuubi/pull/6603?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   


-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "dupen01 (via GitHub)" <gi...@apache.org>.
dupen01 commented on code in PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#discussion_r1713294187


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -80,7 +80,8 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
   hikariConfig.setPoolName("jdbc-metadata-store-pool")
 
   @VisibleForTesting
-  implicit private[kyuubi] val hikariDataSource = new HikariDataSource(hikariConfig)
+  implicit private[kyuubi] val hikariDataSource: HikariDataSource =
+    new HikariDataSource(hikariConfig)

Review Comment:
   With Scala 2.13.11 or above(2.13.9, 2.13.10 not tested), if we use previous code `implicit private[kyuubi] val hikariDataSource = new HikariDataSource(hikariConfig)`,  an error will occur during the compilation phase:
   
   ```
   kyuubi/kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:83: Implicit definition should have explicit type (inferred com.zaxxer.hikari.HikariDataSource) [quickfixable]
   ```
   



-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#issuecomment-2277437795

   the failure should be fixed by upgrade `maven.plugin.silencer.version` from 1.7.13 to 1.7.17


-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#issuecomment-2306187417

   Thanks, merged to master


-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 closed pull request #6603: Upgrade Scala version from 2.13.8 to 2.13.14
URL: https://github.com/apache/kyuubi/pull/6603


-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#discussion_r1726617758


##########
.github/workflows/master.yml:
##########
@@ -129,18 +129,18 @@ jobs:
             **/kyuubi-spark-sql-engine.log*
             **/kyuubi-spark-batch-submit.log*
 
-  scala-test:
-    name: Scala Test
+  java17-scala213-test:
+    name: Java 17 with Scala 2.13 Test
     runs-on: ubuntu-22.04
     strategy:
       fail-fast: false
       matrix:
         scala:
           - '2.13'
         java:
-          - '8'
+          - '21'

Review Comment:
   ```suggestion
             - '17'
   ```



-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#issuecomment-2277467435

   LGTM if CI pass, thanks for your contribution


-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.11 [kyuubi]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#discussion_r1711003184


##########
pom.xml:
##########
@@ -1930,7 +1930,8 @@
             <id>scala-2.13</id>
             <properties>
                 <scala.binary.version>2.13</scala.binary.version>
-                <scala.version>2.13.8</scala.version>
+                <!-- Scala version above 2.13.11 support Java 21. -->

Review Comment:
   keep our dependencies up to date is an regular work, we don't need this comment



##########
pom.xml:
##########
@@ -1930,7 +1930,8 @@
             <id>scala-2.13</id>
             <properties>
                 <scala.binary.version>2.13</scala.binary.version>
-                <scala.version>2.13.8</scala.version>
+                <!-- Scala version above 2.13.11 support Java 21. -->

Review Comment:
   keep our dependencies up to date is a regular work, we don't need this comment



-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "dupen01 (via GitHub)" <gi...@apache.org>.
dupen01 commented on PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#issuecomment-2277452190

   > the failure should be fixed by upgrade `maven.plugin.silencer.version` from 1.7.13 to 1.7.17
   
   yes, I will upgrade `maven.plugin.silencer.version` to 1.7.17


-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] Upgrade Scala version from 2.13.8 to 2.13.14 [kyuubi]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #6603:
URL: https://github.com/apache/kyuubi/pull/6603#discussion_r1713224405


##########
pom.xml:
##########
@@ -241,7 +241,7 @@
         <maven.plugin.jacoco.version>0.8.11</maven.plugin.jacoco.version>
         <maven.plugin.scalastyle.version>1.0.0</maven.plugin.scalastyle.version>
         <maven.plugin.shade.version>3.5.2</maven.plugin.shade.version>
-        <maven.plugin.silencer.version>1.7.13</maven.plugin.silencer.version>
+        <maven.plugin.silencer.version>1.7.17</maven.plugin.silencer.version>

Review Comment:
   each version of silencer has a strictly supported Scala version list, it's common to update with Scala version together
   https://mvnrepository.com/artifact/com.github.ghik/silencer-plugin



-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org