sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Szabolcs Vasas <vasas.szabo...@gmail.com>
Subject Re: Review Request 66067: SQOOP-3052: Introduce gradle-based build for Sqoop to make it more developer friendly / open
Date Thu, 22 Mar 2018 18:29:13 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66067/#review199660
-----------------------------------------------------------



Hi Anna,

Thank you for the patch, it makes the contribution much easier!
I will continue my review tomorrow, I just wanted to post the findings from today.

1) The gradle wrapper script does not find the gradle-wrapper.jar, can you please add it to
the patch as well (https://docs.gradle.org/current/dsl/org.gradle.api.tasks.wrapper.Wrapper.html)?
2) Once the gradle-wrapper.jar is present the ./gradlew test runs successfully
3) The project can be imported to IntelliJ without any problems
4) I think a couple of more items should be added to .gitignore
   .gradle/ -> I think this is generated by the gradle wrapper
   out/ -> I am not sure why but when I import the project to IntelliJ and run a test from
there it generates the out folder which was not present with the ant build
5) Can we define the sourceCompatibility in build.gradle to say that we support 1.7 currently?
6) I have executed the package target with both ant and gradle and I found some differences,
can you please check that these are expected?
   In case of gradle build bat files are also generated for sqoop tools scripts.
   The .gradle folder is copied to build/sqoop-1.5.0-SNAPSHOT.bin__hadoop-2.6.0, I am not
sure this is desired.
   Some library versions are different in case of ant and gradle build.

I am posting the relevant lines from the diff output:
    Only in buildgradle/bin: sqoop-codegen.bat
    Only in buildgradle/bin: sqoop-create-hive-table.bat
    Only in buildgradle/bin: sqoop-eval.bat
    Only in buildgradle/bin: sqoop-export.bat
    Only in buildgradle/bin: sqoop-help.bat
    Only in buildgradle/bin: sqoop-import-all-tables.bat
    Only in buildgradle/bin: sqoop-import-mainframe.bat
    Only in buildgradle/bin: sqoop-import.bat
    Only in buildgradle/bin: sqoop-job.bat
    Only in buildgradle/bin: sqoop-list-databases.bat
    Only in buildgradle/bin: sqoop-list-tables.bat
    Only in buildgradle/bin: sqoop-merge.bat
    Only in buildgradle/bin: sqoop-metastore.bat
    Only in buildgradle/bin: sqoop-version.bat
    Only in build/sqoop-1.5.0-SNAPSHOT.bin__hadoop-2.6.0/.gradle/3.5.1: file-changes
    Binary files build/sqoop-1.5.0-SNAPSHOT.bin__hadoop-2.6.0/.gradle/3.5.1/fileContent/fileContent.lock
and buildgradle/sqoop-1.5.0-SNAPSHOT.bin__hadoop-2.6.0/.gradle/3.5.1/fileContent/fileContent.lock
differ
    Binary files build/sqoop-1.5.0-SNAPSHOT.bin__hadoop-2.6.0/.gradle/3.5.1/taskHistory/fileHashes.bin
and buildgradle/sqoop-1.5.0-SNAPSHOT.bin__hadoop-2.6.0/.gradle/3.5.1/taskHistory/fileHashes.bin
differ
    Binary files build/sqoop-1.5.0-SNAPSHOT.bin__hadoop-2.6.0/.gradle/3.5.1/taskHistory/fileSnapshots.bin
and buildgradle/sqoop-1.5.0-SNAPSHOT.bin__hadoop-2.6.0/.gradle/3.5.1/taskHistory/fileSnapshots.bin
differ
    Binary files build/sqoop-1.5.0-SNAPSHOT.bin__hadoop-2.6.0/.gradle/3.5.1/taskHistory/taskHistory.bin
and buildgradle/sqoop-1.5.0-SNAPSHOT.bin__hadoop-2.6.0/.gradle/3.5.1/taskHistory/taskHistory.bin
differ
    Binary files build/sqoop-1.5.0-SNAPSHOT.bin__hadoop-2.6.0/.gradle/3.5.1/taskHistory/taskHistory.lock
and buildgradle/sqoop-1.5.0-SNAPSHOT.bin__hadoop-2.6.0/.gradle/3.5.1/taskHistory/taskHistory.lock
differ
    Only in buildgradle/sqoop-1.5.0-SNAPSHOT.bin__hadoop-2.6.0/lib: avro-ipc-1.8.1.jar
    Only in build/sqoop-1.5.0-SNAPSHOT.bin__hadoop-2.6.0/lib: commons-codec-1.4.jar
    Only in buildgradle/sqoop-1.5.0-SNAPSHOT.bin__hadoop-2.6.0/lib: commons-codec-1.9.jar
    Only in buildgradle/sqoop-1.5.0-SNAPSHOT.bin__hadoop-2.6.0/lib: parquet-hive-bundle-1.6.0.jar
    Only in build/sqoop-1.5.0-SNAPSHOT.bin__hadoop-2.6.0/lib: slf4j-api-1.6.1.jar
    Only in buildgradle/sqoop-1.5.0-SNAPSHOT.bin__hadoop-2.6.0/lib: slf4j-api-1.7.7.jar


COMPILING.txt
Lines 206 (patched)
<https://reviews.apache.org/r/66067/#comment280038>

    typo: gradlew



COMPILING.txt
Lines 215 (patched)
<https://reviews.apache.org/r/66067/#comment280039>

    typo: gradlew



config/checkstyle/checkstyle-noframes.xsl
Lines 200 (patched)
<https://reviews.apache.org/r/66067/#comment280040>

    There are 2 new lines at the end of this line, git gives a warning when the patch is applied.


- Szabolcs Vasas


On March 19, 2018, 1:55 p.m., Anna Szonyi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66067/
> -----------------------------------------------------------
> 
> (Updated March 19, 2018, 1:55 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: Sqoop-3052
>     https://issues.apache.org/jira/browse/Sqoop-3052
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> SQOOP-3052: Introduce gradle based build for Sqoop to make it more developer friendly
/ open
> 
> 
> Diffs
> -----
> 
>   .gitignore 68cbe28 
>   COMPILING.txt 86be509 
>   build.gradle PRE-CREATION 
>   buildSrc/customUnixStartScript.txt PRE-CREATION 
>   buildSrc/customWindowsStartScript.txt PRE-CREATION 
>   buildSrc/sqoop-package.gradle PRE-CREATION 
>   buildSrc/sqoop-version-gen.gradle PRE-CREATION 
>   config/checkstyle/checkstyle-java-header.txt PRE-CREATION 
>   config/checkstyle/checkstyle-noframes.xsl PRE-CREATION 
>   config/checkstyle/checkstyle.xml PRE-CREATION 
>   gradle.properties PRE-CREATION 
>   gradlew PRE-CREATION 
>   gradlew.bat PRE-CREATION 
>   settings.gradle PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66067/diff/1/
> 
> 
> Testing
> -------
> 
> ran all new tasks, except for internal maven publishing
> 
> Notes:
> - To try it out you can call ./gradlew tasks --all to see all the tasks and compare them
to current tasks/artifacts.
> - Replaced cobertura with jacoco, as it's easier/cleaner to configure, easier to combine
all test results into a single report.
> - Generated pom.xml now has correct dependencies/versions
> - Script generation is currently hardcoded and not based on sqoop help, as previously
- though added the possiblity of hooking it in later
> 
> 
> Thanks,
> 
> Anna Szonyi
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message