QiangCai opened a new pull request #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652 ### Why is this PR needed? there are many redundancy module dependencies ### What changes were proposed in this PR? 1. optimize module dependency 2. fix HiveExample testcase 3. avoid to getOrCreate sparkSession in testcase again 4. add more testcases ### Does this PR introduce any user interface change? - No ### Is any new testcase added? - 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. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
CarbonDataQA1 commented on issue #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#issuecomment-593874863 Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/580/ ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
CarbonDataQA1 commented on issue #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#issuecomment-593910714 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2285/ ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on a change in pull request #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#discussion_r387404541 ########## File path: integration/spark/src/main/scala/org/apache/spark/sql/EnvHelper.scala ########## @@ -26,17 +28,29 @@ object EnvHelper { def isCloud(sparkSession: SparkSession): Boolean = false Review comment: please rename to a more intuitive name ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on a change in pull request #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#discussion_r387406212 ########## File path: integration/spark/src/main/spark2.4/org/apache/spark/sql/parser/SparkSqlAstBuilderWrapper.scala ########## @@ -0,0 +1,11 @@ +package org.apache.spark.sql.parser Review comment: missing license header ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on a change in pull request #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#discussion_r387406240 ########## File path: integration/spark/src/main/spark2.4/org/apache/spark/sql/parser/SparkSqlAstBuilderWrapper.scala ########## @@ -0,0 +1,11 @@ +package org.apache.spark.sql.parser + +import org.apache.spark.sql.catalyst.parser.SqlBaseParser._ +import org.apache.spark.sql.execution.SparkSqlAstBuilder +import org.apache.spark.sql.internal.SQLConf + +class SparkSqlAstBuilderWrapper(conf: SQLConf) Review comment: please add 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. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on a change in pull request #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#discussion_r387406952 ########## File path: sdk/sdk/pom.xml ########## @@ -44,7 +44,6 @@ <dependency> <groupId>org.apache.hadoop</groupId> <artifactId>hadoop-aws</artifactId> - <version>${hadoop.version}</version> Review comment: please remove "build-all" profile also as we build all modules now ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#discussion_r387415365 ########## File path: integration/spark/src/main/scala/org/apache/spark/sql/EnvHelper.scala ########## @@ -26,17 +28,29 @@ object EnvHelper { def isCloud(sparkSession: SparkSession): Boolean = false Review comment: already change to isLegacy ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#discussion_r387415417 ########## File path: integration/spark/src/main/spark2.4/org/apache/spark/sql/parser/SparkSqlAstBuilderWrapper.scala ########## @@ -0,0 +1,11 @@ +package org.apache.spark.sql.parser Review comment: fixed ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#discussion_r387415457 ########## File path: integration/spark/src/main/spark2.4/org/apache/spark/sql/parser/SparkSqlAstBuilderWrapper.scala ########## @@ -0,0 +1,11 @@ +package org.apache.spark.sql.parser + +import org.apache.spark.sql.catalyst.parser.SqlBaseParser._ +import org.apache.spark.sql.execution.SparkSqlAstBuilder +import org.apache.spark.sql.internal.SQLConf + +class SparkSqlAstBuilderWrapper(conf: SQLConf) Review comment: added ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#discussion_r387415486 ########## File path: sdk/sdk/pom.xml ########## @@ -44,7 +44,6 @@ <dependency> <groupId>org.apache.hadoop</groupId> <artifactId>hadoop-aws</artifactId> - <version>${hadoop.version}</version> 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
CarbonDataQA1 commented on issue #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#issuecomment-594294770 Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/592/ ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
CarbonDataQA1 commented on issue #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#issuecomment-594317071 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2299/ ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on issue #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#issuecomment-594326592 LGTM ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
Indhumathi27 commented on a change in pull request #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#discussion_r387450269 ########## File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/cloud/CacheRefreshTestCase.scala ########## @@ -0,0 +1,36 @@ +package org.apache.carbondata.spark.testsuite.cloud Review comment: Please Add License ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
Indhumathi27 commented on a change in pull request #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#discussion_r387450880 ########## File path: integration/spark/src/test/scala/org/apache/spark/carbondata/TestStreamingTableOpName.scala ########## @@ -2079,6 +2079,9 @@ class TestStreamingTableOpName extends QueryTest with BeforeAndAfterAll { Thread.sleep(continueSeconds * 1000) thread2.interrupt() thread1.interrupt() + } catch { + case ex => + LOGGER.error("finished to ingest data", ex) Review comment: Please change the error message, as it represents a failure ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
Indhumathi27 commented on a change in pull request #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#discussion_r387450914 ########## File path: integration/spark/src/test/scala/org/apache/spark/carbondata/TestStreamingTableQueryFilter.scala ########## @@ -240,6 +240,9 @@ class TestStreamingTableQueryFilter extends QueryTest with BeforeAndAfterAll { Thread.sleep(continueSeconds * 1000) thread2.interrupt() thread1.interrupt() + } catch { + case ex => + LOGGER.error("finished to ingest data", ex) Review comment: Please change the error message, as it represents a failure ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
Indhumathi27 commented on a change in pull request #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#discussion_r387450942 ########## File path: integration/spark/src/test/scala/org/apache/spark/carbondata/TestStreamingTableWithLongString.scala ########## @@ -478,6 +478,9 @@ class TestStreamingTableWithLongString extends QueryTest with BeforeAndAfterAll Thread.sleep(continueSeconds * 1000) thread2.interrupt() thread1.interrupt() + } catch { + case ex => + LOGGER.error("finished to ingest data", ex) Review comment: Please change the error message, as it represents a failure ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
Indhumathi27 commented on a change in pull request #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#discussion_r387450980 ########## File path: integration/spark/src/test/scala/org/apache/spark/carbondata/TestStreamingTableWithRowParser.scala ########## @@ -860,6 +860,9 @@ class TestStreamingTableWithRowParser extends QueryTest with BeforeAndAfterAll { Thread.sleep(continueSeconds * 1000) thread2.interrupt() thread1.interrupt() + } catch { + case ex => + LOGGER.error("finished to ingest data", ex) Review comment: Please change the error message, as it represents a failure ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
Indhumathi27 commented on a change in pull request #3652: [HOTFIX] optimize module dependency
URL: https://github.com/apache/carbondata/pull/3652#discussion_r387450269 ########## File path: integration/spark/src/test/scala/org/apache/carbondata/spark/testsuite/cloud/CacheRefreshTestCase.scala ########## @@ -0,0 +1,36 @@ +package org.apache.carbondata.spark.testsuite.cloud Review comment: Please Add License Header ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
Free forum by Nabble | Edit this page |