[GitHub] carbondata pull request #2920: [HOTFIX] Improve log message in CarbonWriterB...

classic Classic list List threaded Threaded
21 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #2920: [HOTFIX] Improve log message in CarbonWriterB...

qiuchenjian-2
GitHub user jackylk opened a pull request:

    https://github.com/apache/carbondata/pull/2920

    [HOTFIX] Improve log message in CarbonWriterBuilder

    In master the log message is not proper:
    `AppName is not set, please use writtenBy() API to set the App Namewhich is using SDK`
   
    This PR improves log message in CarbonWriterBuilder
   
     - [X] Any interfaces changed?
     No
     - [X] Any backward compatibility impacted?
     No
     - [X] Document update required?
    No
     - [X] Testing done
            Please provide details on
            - Whether new unit test cases have been added or why no new tests are required?
            - How it is tested? Please attach test report.
            - Is it a performance related change? Please attach the performance test report.
            - Any additional information to help reviewers in testing this change.
     rerun all test      
     - [X] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
    NA

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

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

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

    https://github.com/apache/carbondata/pull/2920.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 #2920
   
----
commit b7fd6a6a9469feb99fdbc6b82907bc5c57381792
Author: Jacky Li <jacky.likun@...>
Date:   2018-11-14T12:54:33Z

    Update CarbonWriterBuilder.java

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #2920: [HOTFIX] Improve log message in CarbonWriterBuilder

qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2920
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1614/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #2920: [HOTFIX] Improve log message in CarbonWriterBuilder

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2920
 
    Build Failed  with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9662/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #2920: [HOTFIX] Improve log message in CarbonWriterBuilder

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2920
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1404/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #2920: [HOTFIX] Improve log message in CarbonWriterB...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2920#discussion_r233437690
 
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonWriterBuilder.java ---
    @@ -438,13 +438,13 @@ public CarbonWriter build() throws IOException, InvalidLoadOptionException {
         Objects.requireNonNull(path, "path should not be null");
         if (this.writerType == null) {
           throw new IOException(
    -          "Writer type is not set, use withCsvInput() or withAvroInput() or withJsonInput()  "
    +          "'writerType' must be set, use withCsvInput() or withAvroInput() or withJsonInput()  "
                   + "API based on input");
         }
         if (this.writtenByApp == null || this.writtenByApp.isEmpty()) {
           throw new RuntimeException(
    -          "AppName is not set, please use writtenBy() API to set the App Name"
    -              + "which is using SDK");
    +          "'writtenBy' must be set when writting carbon files, use writtenBy() API to "
    --- End diff --
   
    ```suggestion
              "'writtenBy' must be set when writting carbon files, use writtenBy() API to "
    ```


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #2920: [HOTFIX] Improve log message in CarbonWriterBuilder

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2920
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1405/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #2920: [HOTFIX] Improve log message in CarbonWriterB...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user xuchuanyin commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2920#discussion_r233458367
 
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonWriterBuilder.java ---
    @@ -438,13 +438,13 @@ public CarbonWriter build() throws IOException, InvalidLoadOptionException {
         Objects.requireNonNull(path, "path should not be null");
         if (this.writerType == null) {
           throw new IOException(
    -          "Writer type is not set, use withCsvInput() or withAvroInput() or withJsonInput()  "
    +          "'writerType' must be set, use withCsvInput() or withAvroInput() or withJsonInput()  "
                   + "API based on input");
         }
         if (this.writtenByApp == null || this.writtenByApp.isEmpty()) {
           throw new RuntimeException(
    -          "AppName is not set, please use writtenBy() API to set the App Name"
    -              + "which is using SDK");
    +          "'writtenBy' must be set when writting carbon files, use writtenBy() API to "
    --- End diff --
   
    writting --> writing


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #2920: [HOTFIX] Improve log message in CarbonWriterB...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user xuchuanyin commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2920#discussion_r233458646
 
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonWriterBuilder.java ---
    @@ -438,13 +438,13 @@ public CarbonWriter build() throws IOException, InvalidLoadOptionException {
         Objects.requireNonNull(path, "path should not be null");
         if (this.writerType == null) {
           throw new IOException(
    -          "Writer type is not set, use withCsvInput() or withAvroInput() or withJsonInput()  "
    +          "'writerType' must be set, use withCsvInput() or withAvroInput() or withJsonInput()  "
                   + "API based on input");
         }
         if (this.writtenByApp == null || this.writtenByApp.isEmpty()) {
           throw new RuntimeException(
    -          "AppName is not set, please use writtenBy() API to set the App Name"
    -              + "which is using SDK");
    +          "'writtenBy' must be set when writting carbon files, use writtenBy() API to "
    +              + "set it, it can be the application name which using the SDK");
    --- End diff --
   
    it can be the name of the application which uses the SDK.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #2920: [HOTFIX] Improve log message in CarbonWriterBuilder

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2920
 
    Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1615/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #2920: [HOTFIX] Improve log message in CarbonWriterBuilder

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2920
 
    Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9663/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #2920: [HOTFIX] Improve log message in CarbonWriterB...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2920#discussion_r233470872
 
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonWriterBuilder.java ---
    @@ -438,13 +438,13 @@ public CarbonWriter build() throws IOException, InvalidLoadOptionException {
         Objects.requireNonNull(path, "path should not be null");
         if (this.writerType == null) {
           throw new IOException(
    -          "Writer type is not set, use withCsvInput() or withAvroInput() or withJsonInput()  "
    +          "'writerType' must be set, use withCsvInput() or withAvroInput() or withJsonInput()  "
                   + "API based on input");
         }
         if (this.writtenByApp == null || this.writtenByApp.isEmpty()) {
           throw new RuntimeException(
    -          "AppName is not set, please use writtenBy() API to set the App Name"
    -              + "which is using SDK");
    +          "'writtenBy' must be set when writting carbon files, use writtenBy() API to "
    +              + "set it, it can be the application name which using the SDK");
    --- End diff --
   
    ```suggestion
                  + "set it, it can be the name of the application which using the SDK");
    ```


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #2920: [HOTFIX] Improve log message in CarbonWriterBuilder

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2920
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1406/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #2920: [HOTFIX] Improve log message in CarbonWriterBuilder

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2920
 
    Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1616/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #2920: [HOTFIX] Improve log message in CarbonWriterBuilder

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2920
 
    Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9664/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #2920: [HOTFIX] Improve log message in CarbonWriterB...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user xuchuanyin commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2920#discussion_r234098865
 
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonWriterBuilder.java ---
    @@ -438,13 +438,13 @@ public CarbonWriter build() throws IOException, InvalidLoadOptionException {
         Objects.requireNonNull(path, "path should not be null");
         if (this.writerType == null) {
           throw new IOException(
    -          "Writer type is not set, use withCsvInput() or withAvroInput() or withJsonInput()  "
    +          "'writerType' must be set, use withCsvInput() or withAvroInput() or withJsonInput()  "
                   + "API based on input");
         }
         if (this.writtenByApp == null || this.writtenByApp.isEmpty()) {
           throw new RuntimeException(
    -          "AppName is not set, please use writtenBy() API to set the App Name"
    -              + "which is using SDK");
    +          "'writtenBy' must be set when writting carbon files, use writtenBy() API to "
    --- End diff --
   
    ```suggestion
              "'writtenBy' must be set when writing carbon files, use writtenBy() API to "
    ```


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #2920: [HOTFIX] Improve log message in CarbonWriterBuilder

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2920
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1435/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #2920: [HOTFIX] Improve log message in CarbonWriterBuilder

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user xuchuanyin commented on the issue:

    https://github.com/apache/carbondata/pull/2920
 
    LGTM
    Waiting for the builds


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #2920: [HOTFIX] Improve log message in CarbonWriterBuilder

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2920
 
    Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9693/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #2920: [HOTFIX] Improve log message in CarbonWriterBuilder

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2920
 
    Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1645/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #2920: [HOTFIX] Improve log message in CarbonWriterBuilder

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2920
 
    LGTM


---
12