[GitHub] incubator-carbondata pull request #365: [CARBONDATA-463] Extract code to spa...

[qiuchenjian-2]

GitHub user jackylk opened a pull request:

    https://github.com/apache/incubator-carbondata/pull/365

    [CARBONDATA-463] Extract code to spark-common

    Move MM

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jackylk/incubator-carbondata sparkcommon

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-carbondata/pull/365.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #365
   
----
commit ab8cfd0eaf66ae74890539d6cb942bd980dae957
Author: jackylk <[hidden email]>
Date:   2016-11-28T15:51:09Z

    extract code to spark-common

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user jackylk commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/365
 
    http://136.243.101.176:8080/job/ApacheCarbonManualPRBuilder/689/


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user scwf commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/365
 
    i think we should merge this into our spark2 branch, otherwise it will leads to large conflicts


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user jackylk commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/365
 
    @scwf
    I have compared two branches and extracted out the common files, when we push spark2 integration code, common files will be excluded. I think in this way the the total effort will be less.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user jackylk commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/365
 
    CI passed
    http://136.243.101.176:8080/job/ApacheCarbonManualPRBuilder/696/


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/365#discussion_r90040864
 
    --- Diff: integration/spark-common/src/main/java/org/apache/carbondata/integration/spark/merger/CarbonCompactionUtil.java ---
    @@ -38,7 +38,6 @@
     import org.apache.carbondata.core.util.CarbonUtil;
     import org.apache.carbondata.core.util.CarbonUtilException;
     
    --- End diff --
   
    I guess better remove `integration` from package for all classes


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/365#discussion_r90041752
 
    --- Diff: integration/spark/src/main/scala/org/apache/carbondata/spark/rdd/NewCarbonDataLoadRDD.scala ---
    @@ -51,7 +51,6 @@ class NewCarbonDataLoadRDD[K, V](
         sc: SparkContext,
    --- End diff --
   
    Can't we move this to common module.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/365#discussion_r90044286
 
    --- Diff: integration/spark/src/main/scala/org/apache/carbondata/spark/rdd/NewCarbonDataLoadRDD.scala ---
    @@ -51,7 +51,6 @@ class NewCarbonDataLoadRDD[K, V](
         sc: SparkContext,
    --- End diff --
   
    I think we can try it later. Currently because of newTaskAttemptID and some other function signature is changed in spark2, so need to duplicate code temporarily



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/365#discussion_r90044359
 
    --- Diff: integration/spark-common/src/main/java/org/apache/carbondata/integration/spark/merger/CarbonCompactionUtil.java ---
    @@ -38,7 +38,6 @@
     import org.apache.carbondata.core.util.CarbonUtil;
     import org.apache.carbondata.core.util.CarbonUtilException;
     
    --- End diff --
   
    ok


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/365
 
    LGTM except few comments.
    It seems there is an issue in pom files while using maven default build and using format. I will fix them in new PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/365#discussion_r90045692
 
    --- Diff: integration/spark-common/src/main/java/org/apache/carbondata/integration/spark/merger/CarbonCompactionUtil.java ---
    @@ -38,7 +38,6 @@
     import org.apache.carbondata.core.util.CarbonUtil;
     import org.apache.carbondata.core.util.CarbonUtilException;
     
    --- End diff --
   
    fixed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/365
 
    LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-carbondata/pull/365


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---