You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/12/23 02:22:49 UTC

[GitHub] [kyuubi] lightning-L opened a new pull request, #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

lightning-L opened a new pull request, #4022:
URL: https://github.com/apache/kyuubi/pull/4022

   <!--
   Thanks for sending a pull request!
   
   Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/CONTRIBUTING.html
     2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
     3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
   -->
   
   ### _Why are the changes needed?_
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you add a feature, you can talk about the use case of it.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   
   
   ### _How was this patch tested?_
   - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
   
   - [ ] Add screenshots for manual tests if appropriate
   
   - [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
   


-- 
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


[GitHub] [kyuubi] turboFei commented on a diff in pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#discussion_r1062123441


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -78,17 +79,17 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
 
   private def initSchema(): Unit = {
     val classLoader = Utils.getContextOrKyuubiClassLoader
-    val initSchemaStream: Option[InputStream] = dbType match {
-      case DERBY =>
-        Option(classLoader.getResourceAsStream("sql/derby/metadata-store-schema-derby.sql"))
-      case MYSQL =>
-        Option(classLoader.getResourceAsStream("sql/mysql/metadata-store-schema-mysql.sql"))
+    val initSchemaFile: Option[File] = dbType match {
+      case DERBY | MYSQL =>
+        getInitSchemaStream(classLoader.getResource(s"sql/${dbType.toString.toLowerCase}"))
       case CUSTOM => None
     }
-    initSchemaStream.foreach { inputStream =>
+    initSchemaFile.foreach { schemaFile =>
+      info(s"Init schema file: ${schemaFile.getAbsolutePath}")
+      val reader = new BufferedReader(new FileReader(schemaFile))

Review Comment:
   FYI:
   
   https://github.com/lightning-L/incubator-kyuubi/pull/1/files
   
   
   You can use FileReader for UT, but for binary, it is not a File, it is just a Resource.



-- 
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


[GitHub] [kyuubi] pan3793 commented on a diff in pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#discussion_r1059222193


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -78,11 +78,14 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
 
   private def initSchema(): Unit = {
     val classLoader = Utils.getContextOrKyuubiClassLoader
+    val kyuubiShortVersion = KYUUBI_VERSION.split("-")(0)
     val initSchemaStream: Option[InputStream] = dbType match {
       case DERBY =>
-        Option(classLoader.getResourceAsStream("sql/derby/metadata-store-schema-derby.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/derby/metadata-store-schema-${kyuubiShortVersion}.derby.sql"))
       case MYSQL =>
-        Option(classLoader.getResourceAsStream("sql/mysql/metadata-store-schema-mysql.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/mysql/metadata-store-schema-${kyuubiShortVersion}.mysql.sql"))

Review Comment:
   Another way is listing all schema files and always using the greatest 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


[GitHub] [kyuubi] ulysses-you commented on a diff in pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#discussion_r1057452318


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -78,11 +78,14 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
 
   private def initSchema(): Unit = {
     val classLoader = Utils.getContextOrKyuubiClassLoader
+    val kyuubiShortVersion = KYUUBI_VERSION.split("-")(0)
     val initSchemaStream: Option[InputStream] = dbType match {
       case DERBY =>
-        Option(classLoader.getResourceAsStream("sql/derby/metadata-store-schema-derby.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/derby/metadata-store-schema-${kyuubiShortVersion}.derby.sql"))
       case MYSQL =>
-        Option(classLoader.getResourceAsStream("sql/mysql/metadata-store-schema-mysql.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/mysql/metadata-store-schema-${kyuubiShortVersion}.mysql.sql"))

Review Comment:
   we can have base file + delta file. Then base file do not need version. delta file should have version information to specify which version cause the changes.



-- 
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


[GitHub] [kyuubi] turboFei commented on a diff in pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#discussion_r1058724293


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -78,11 +78,14 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
 
   private def initSchema(): Unit = {
     val classLoader = Utils.getContextOrKyuubiClassLoader
+    val kyuubiShortVersion = KYUUBI_VERSION.split("-")(0)
     val initSchemaStream: Option[InputStream] = dbType match {
       case DERBY =>
-        Option(classLoader.getResourceAsStream("sql/derby/metadata-store-schema-derby.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/derby/metadata-store-schema-${kyuubiShortVersion}.derby.sql"))
       case MYSQL =>
-        Option(classLoader.getResourceAsStream("sql/mysql/metadata-store-schema-mysql.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/mysql/metadata-store-schema-${kyuubiShortVersion}.mysql.sql"))

Review Comment:
   how do you think about @pan3793 ?



-- 
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


[GitHub] [kyuubi] turboFei commented on pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#issuecomment-1374392710

   thanks @lightning-L , merged to master
   
   please raise a pr to followup the `version` sort .


-- 
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


[GitHub] [kyuubi] turboFei commented on a diff in pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#discussion_r1063003583


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -78,17 +79,17 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
 
   private def initSchema(): Unit = {
     val classLoader = Utils.getContextOrKyuubiClassLoader
-    val initSchemaStream: Option[InputStream] = dbType match {
-      case DERBY =>
-        Option(classLoader.getResourceAsStream("sql/derby/metadata-store-schema-derby.sql"))
-      case MYSQL =>
-        Option(classLoader.getResourceAsStream("sql/mysql/metadata-store-schema-mysql.sql"))
+    val initSchemaFile: Option[File] = dbType match {
+      case DERBY | MYSQL =>
+        getInitSchemaStream(classLoader.getResource(s"sql/${dbType.toString.toLowerCase}"))
       case CUSTOM => None
     }
-    initSchemaStream.foreach { inputStream =>
+    initSchemaFile.foreach { schemaFile =>
+      info(s"Init schema file: ${schemaFile.getAbsolutePath}")
+      val reader = new BufferedReader(new FileReader(schemaFile))

Review Comment:
   this is another follow up: https://github.com/apache/kyuubi/pull/4103
   
   you can merge the https://github.com/lightning-L/incubator-kyuubi/pull/2/files



-- 
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


[GitHub] [kyuubi] turboFei commented on a diff in pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#discussion_r1062106079


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -78,17 +79,17 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
 
   private def initSchema(): Unit = {
     val classLoader = Utils.getContextOrKyuubiClassLoader
-    val initSchemaStream: Option[InputStream] = dbType match {
-      case DERBY =>
-        Option(classLoader.getResourceAsStream("sql/derby/metadata-store-schema-derby.sql"))
-      case MYSQL =>
-        Option(classLoader.getResourceAsStream("sql/mysql/metadata-store-schema-mysql.sql"))
+    val initSchemaFile: Option[File] = dbType match {
+      case DERBY | MYSQL =>
+        getInitSchemaStream(classLoader.getResource(s"sql/${dbType.toString.toLowerCase}"))
       case CUSTOM => None
     }
-    initSchemaStream.foreach { inputStream =>
+    initSchemaFile.foreach { schemaFile =>
+      info(s"Init schema file: ${schemaFile.getAbsolutePath}")
+      val reader = new BufferedReader(new FileReader(schemaFile))

Review Comment:
   will raise a pr to followup it, we have to use inputStream instead of FileReader



-- 
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


[GitHub] [kyuubi] turboFei commented on pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#issuecomment-1364874453

   ```
    Failed to execute goal org.apache.rat:apache-rat-plugin:0.13:check (default-cli) on project kyuubi-parent: Too many files with unapproved license: 4 See RAT report in: /home/runner/work/kyuubi/kyuubi/target/rat.txt -> [Help 1]
   org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.rat:apache-rat-plugin:0.13:check (default-cli) on project kyuubi-parent: Too many files with unapproved license: 4 See RAT report in: /home/runner/work/kyuubi/kyuubi/target/rat.txt
   ```


-- 
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


[GitHub] [kyuubi] turboFei commented on pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#issuecomment-1365606513

   > ```
   > AdminRestApiSuite:
   > *** RUN ABORTED ***
   >   org.apache.kyuubi.KyuubiException: Failed to Start KyuubiServer
   >   at org.apache.kyuubi.service.CompositeService.$anonfun$start$1(CompositeService.scala:52)
   >   at org.apache.kyuubi.service.CompositeService.$anonfun$start$1$adapted(CompositeService.scala:45)
   >   at scala.collection.mutable.ResizableArray.foreach(ResizableArray.scala:62)
   >   at scala.collection.mutable.ResizableArray.foreach$(ResizableArray.scala:55)
   >   at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:49)
   >   at org.apache.kyuubi.service.CompositeService.start(CompositeService.scala:45)
   >   at org.apache.kyuubi.service.Serverable.start(Serverable.scala:51)
   >   at org.apache.kyuubi.server.KyuubiServer.start(KyuubiServer.scala:147)
   >   at org.apache.kyuubi.server.KyuubiServer$.startServer(KyuubiServer.scala:65)
   >   at org.apache.kyuubi.WithKyuubiServer.beforeAll(WithKyuubiServer.scala:62)
   >   ...
   >   Cause: org.apache.kyuubi.KyuubiException: Cannot start KyuubiRestFrontendService
   > ```
   > 
   > https://github.com/apache/kyuubi/actions/runs/3779542104/jobs/6424890802
   > 
   > FYI: you can run the UT locally, You can also check the GA log(click the upper left corner summary button). <img alt="image" width="1727" src="https://user-images.githubusercontent.com/6757692/209590953-2d9c8bd2-2dfc-412c-8f3c-994f58fbb214.png">
   
   The ut failure does not make sense.
   
   would you like to rebase the code and have a try?


-- 
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


[GitHub] [kyuubi] turboFei commented on a diff in pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#discussion_r1057433924


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -78,11 +78,14 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
 
   private def initSchema(): Unit = {
     val classLoader = Utils.getContextOrKyuubiClassLoader
+    val kyuubiShortVersion = KYUUBI_VERSION.split("-")(0)
     val initSchemaStream: Option[InputStream] = dbType match {
       case DERBY =>
-        Option(classLoader.getResourceAsStream("sql/derby/metadata-store-schema-derby.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/derby/metadata-store-schema-${kyuubiShortVersion}.derby.sql"))
       case MYSQL =>
-        Option(classLoader.getResourceAsStream("sql/mysql/metadata-store-schema-mysql.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/mysql/metadata-store-schema-${kyuubiShortVersion}.mysql.sql"))

Review Comment:
   seems difficult to maintain this resource file.
   
   we have to change the resource file name when bumping the project 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


[GitHub] [kyuubi] turboFei commented on a diff in pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#discussion_r1057434946


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -78,11 +78,14 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
 
   private def initSchema(): Unit = {
     val classLoader = Utils.getContextOrKyuubiClassLoader
+    val kyuubiShortVersion = KYUUBI_VERSION.split("-")(0)
     val initSchemaStream: Option[InputStream] = dbType match {
       case DERBY =>
-        Option(classLoader.getResourceAsStream("sql/derby/metadata-store-schema-derby.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/derby/metadata-store-schema-${kyuubiShortVersion}.derby.sql"))
       case MYSQL =>
-        Option(classLoader.getResourceAsStream("sql/mysql/metadata-store-schema-mysql.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/mysql/metadata-store-schema-${kyuubiShortVersion}.mysql.sql"))

Review Comment:
   is it possible to use a soft link 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


[GitHub] [kyuubi] pan3793 commented on a diff in pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#discussion_r1059221968


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -78,11 +78,14 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
 
   private def initSchema(): Unit = {
     val classLoader = Utils.getContextOrKyuubiClassLoader
+    val kyuubiShortVersion = KYUUBI_VERSION.split("-")(0)
     val initSchemaStream: Option[InputStream] = dbType match {
       case DERBY =>
-        Option(classLoader.getResourceAsStream("sql/derby/metadata-store-schema-derby.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/derby/metadata-store-schema-${kyuubiShortVersion}.derby.sql"))
       case MYSQL =>
-        Option(classLoader.getResourceAsStream("sql/mysql/metadata-store-schema-mysql.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/mysql/metadata-store-schema-${kyuubiShortVersion}.mysql.sql"))

Review Comment:
   I prefer to dynamically calculate the schema version, but the current calculation is not correct, think about 1.7.1, the expected result is 1.7.0



-- 
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


[GitHub] [kyuubi] turboFei commented on pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#issuecomment-1364875977

   thanks for driving this @lightning-L 
   
   could you add apache license header in the new added files?
   


-- 
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


[GitHub] [kyuubi] turboFei commented on pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#issuecomment-1365523769

   ```
   AdminRestApiSuite:
   *** RUN ABORTED ***
     org.apache.kyuubi.KyuubiException: Failed to Start KyuubiServer
     at org.apache.kyuubi.service.CompositeService.$anonfun$start$1(CompositeService.scala:52)
     at org.apache.kyuubi.service.CompositeService.$anonfun$start$1$adapted(CompositeService.scala:45)
     at scala.collection.mutable.ResizableArray.foreach(ResizableArray.scala:62)
     at scala.collection.mutable.ResizableArray.foreach$(ResizableArray.scala:55)
     at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:49)
     at org.apache.kyuubi.service.CompositeService.start(CompositeService.scala:45)
     at org.apache.kyuubi.service.Serverable.start(Serverable.scala:51)
     at org.apache.kyuubi.server.KyuubiServer.start(KyuubiServer.scala:147)
     at org.apache.kyuubi.server.KyuubiServer$.startServer(KyuubiServer.scala:65)
     at org.apache.kyuubi.WithKyuubiServer.beforeAll(WithKyuubiServer.scala:62)
     ...
     Cause: org.apache.kyuubi.KyuubiException: Cannot start KyuubiRestFrontendService
   ```


-- 
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


[GitHub] [kyuubi] turboFei commented on a diff in pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#discussion_r1057433395


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -78,11 +78,14 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
 
   private def initSchema(): Unit = {
     val classLoader = Utils.getContextOrKyuubiClassLoader
+    val kyuubiShortVersion = KYUUBI_VERSION.split("-")(0)

Review Comment:
   val kyuubiShortVersion = KYUUBI_VERSION.split("-").head



-- 
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


[GitHub] [kyuubi] turboFei commented on a diff in pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#discussion_r1057434469


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -78,11 +78,14 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
 
   private def initSchema(): Unit = {
     val classLoader = Utils.getContextOrKyuubiClassLoader
+    val kyuubiShortVersion = KYUUBI_VERSION.split("-")(0)
     val initSchemaStream: Option[InputStream] = dbType match {
       case DERBY =>
-        Option(classLoader.getResourceAsStream("sql/derby/metadata-store-schema-derby.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/derby/metadata-store-schema-${kyuubiShortVersion}.derby.sql"))
       case MYSQL =>
-        Option(classLoader.getResourceAsStream("sql/mysql/metadata-store-schema-mysql.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/mysql/metadata-store-schema-${kyuubiShortVersion}.mysql.sql"))

Review Comment:
   do you have some suggestions? @pan3793 



-- 
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


[GitHub] [kyuubi] lightning-L commented on a diff in pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
lightning-L commented on code in PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#discussion_r1057467301


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -78,11 +78,14 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
 
   private def initSchema(): Unit = {
     val classLoader = Utils.getContextOrKyuubiClassLoader
+    val kyuubiShortVersion = KYUUBI_VERSION.split("-")(0)
     val initSchemaStream: Option[InputStream] = dbType match {
       case DERBY =>
-        Option(classLoader.getResourceAsStream("sql/derby/metadata-store-schema-derby.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/derby/metadata-store-schema-${kyuubiShortVersion}.derby.sql"))
       case MYSQL =>
-        Option(classLoader.getResourceAsStream("sql/mysql/metadata-store-schema-mysql.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/mysql/metadata-store-schema-${kyuubiShortVersion}.mysql.sql"))

Review Comment:
   no we cannot use symbolic link, maven will fail.
   
   [ERROR] Failed to execute goal org.apache.maven.plugins:maven-resources-plugin:3.2.0:resources (default-resources) on project kyuubi-server_2.12: /Users/tiliao/Documents/Code/incubator-kyuubi/kyuubi-server/target/scala-2.12/classes/sql/derby/metadata-store-schema-derby.sql -> [Help 1]
   org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-resources-plugin:3.2.0:resources (default-resources) on project kyuubi-server_2.12: /Users/tiliao/Documents/Code/incubator-kyuubi/kyuubi-server/target/scala-2.12/classes/sql/derby/metadata-store-schema-derby.sql
   
   
   Caused by: org.apache.maven.shared.filtering.MavenFilteringException: /Users/tiliao/Documents/Code/incubator-kyuubi/kyuubi-server/target/scala-2.12/classes/sql/derby/metadata-store-schema-derby.sql
       at org.apache.maven.shared.filtering.DefaultMavenFileFilter.copyFile (DefaultMavenFileFilter.java:113)
       at org.apache.maven.shared.filtering.DefaultMavenResourcesFiltering.filterResources (DefaultMavenResourcesFiltering.java:262)
   
   
   https://issues.apache.org/jira/browse/MRESOURCES-269



-- 
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


[GitHub] [kyuubi] turboFei commented on a diff in pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#discussion_r1062123441


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -78,17 +79,17 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
 
   private def initSchema(): Unit = {
     val classLoader = Utils.getContextOrKyuubiClassLoader
-    val initSchemaStream: Option[InputStream] = dbType match {
-      case DERBY =>
-        Option(classLoader.getResourceAsStream("sql/derby/metadata-store-schema-derby.sql"))
-      case MYSQL =>
-        Option(classLoader.getResourceAsStream("sql/mysql/metadata-store-schema-mysql.sql"))
+    val initSchemaFile: Option[File] = dbType match {
+      case DERBY | MYSQL =>
+        getInitSchemaStream(classLoader.getResource(s"sql/${dbType.toString.toLowerCase}"))
       case CUSTOM => None
     }
-    initSchemaStream.foreach { inputStream =>
+    initSchemaFile.foreach { schemaFile =>
+      info(s"Init schema file: ${schemaFile.getAbsolutePath}")
+      val reader = new BufferedReader(new FileReader(schemaFile))

Review Comment:
   FYI:
   
   https://github.com/lightning-L/incubator-kyuubi/pull/1/files
   
   
   You can use FileReader for UT, but for binary, there is no File, there is just Resource.



-- 
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


[GitHub] [kyuubi] lightning-L commented on a diff in pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
lightning-L commented on code in PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#discussion_r1057448707


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -78,11 +78,14 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
 
   private def initSchema(): Unit = {
     val classLoader = Utils.getContextOrKyuubiClassLoader
+    val kyuubiShortVersion = KYUUBI_VERSION.split("-")(0)
     val initSchemaStream: Option[InputStream] = dbType match {
       case DERBY =>
-        Option(classLoader.getResourceAsStream("sql/derby/metadata-store-schema-derby.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/derby/metadata-store-schema-${kyuubiShortVersion}.derby.sql"))
       case MYSQL =>
-        Option(classLoader.getResourceAsStream("sql/mysql/metadata-store-schema-mysql.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/mysql/metadata-store-schema-${kyuubiShortVersion}.mysql.sql"))

Review Comment:
   yes, we can use soft link



-- 
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


[GitHub] [kyuubi] pan3793 commented on a diff in pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#discussion_r1059222193


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -78,11 +78,14 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
 
   private def initSchema(): Unit = {
     val classLoader = Utils.getContextOrKyuubiClassLoader
+    val kyuubiShortVersion = KYUUBI_VERSION.split("-")(0)
     val initSchemaStream: Option[InputStream] = dbType match {
       case DERBY =>
-        Option(classLoader.getResourceAsStream("sql/derby/metadata-store-schema-derby.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/derby/metadata-store-schema-${kyuubiShortVersion}.derby.sql"))
       case MYSQL =>
-        Option(classLoader.getResourceAsStream("sql/mysql/metadata-store-schema-mysql.sql"))
+        Option(classLoader
+          .getResourceAsStream(s"sql/mysql/metadata-store-schema-${kyuubiShortVersion}.mysql.sql"))

Review Comment:
   Another way is listing all schema files and only always using the greatest 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


[GitHub] [kyuubi] turboFei commented on a diff in pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#discussion_r1063028836


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -78,17 +79,17 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
 
   private def initSchema(): Unit = {
     val classLoader = Utils.getContextOrKyuubiClassLoader
-    val initSchemaStream: Option[InputStream] = dbType match {
-      case DERBY =>
-        Option(classLoader.getResourceAsStream("sql/derby/metadata-store-schema-derby.sql"))
-      case MYSQL =>
-        Option(classLoader.getResourceAsStream("sql/mysql/metadata-store-schema-mysql.sql"))
+    val initSchemaFile: Option[File] = dbType match {
+      case DERBY | MYSQL =>
+        getInitSchemaStream(classLoader.getResource(s"sql/${dbType.toString.toLowerCase}"))
       case CUSTOM => None
     }
-    initSchemaStream.foreach { inputStream =>
+    initSchemaFile.foreach { schemaFile =>
+      info(s"Init schema file: ${schemaFile.getAbsolutePath}")
+      val reader = new BufferedReader(new FileReader(schemaFile))

Review Comment:
   the UT in https://github.com/apache/kyuubi/pull/4103 has passed. @lightning-L 
   



-- 
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


[GitHub] [kyuubi] turboFei closed pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
turboFei closed pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc
URL: https://github.com/apache/kyuubi/pull/4022


-- 
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


[GitHub] [kyuubi] turboFei commented on a diff in pull request #4022: [KYUUBI #3968] Upgrading and migration script for Jdbc

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #4022:
URL: https://github.com/apache/kyuubi/pull/4022#discussion_r1063024572


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala:
##########
@@ -78,17 +79,17 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
 
   private def initSchema(): Unit = {
     val classLoader = Utils.getContextOrKyuubiClassLoader
-    val initSchemaStream: Option[InputStream] = dbType match {
-      case DERBY =>
-        Option(classLoader.getResourceAsStream("sql/derby/metadata-store-schema-derby.sql"))
-      case MYSQL =>
-        Option(classLoader.getResourceAsStream("sql/mysql/metadata-store-schema-mysql.sql"))
+    val initSchemaFile: Option[File] = dbType match {
+      case DERBY | MYSQL =>
+        getInitSchemaStream(classLoader.getResource(s"sql/${dbType.toString.toLowerCase}"))
       case CUSTOM => None
     }
-    initSchemaStream.foreach { inputStream =>
+    initSchemaFile.foreach { schemaFile =>
+      info(s"Init schema file: ${schemaFile.getAbsolutePath}")
+      val reader = new BufferedReader(new FileReader(schemaFile))

Review Comment:
   I refer this: https://stackoverflow.com/questions/1429172/how-to-list-the-files-inside-a-jar-file/28057735#28057735
   



-- 
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