Re: Review Request 54528: SQOOP-3042 - Sqoop does not clear compile directory under /tmp/sqoop-/compile automatically

[Eric Lin <er...@cloudera.com>]

> On Dec. 12, 2016, 8:57 a.m., Szabolcs Vasas wrote:
> > src/java/org/apache/sqoop/orm/ClassWriter.java
> > Lines 1778 (patched)
> > <https://reviews.apache.org/r/54528/diff/1/?file=1579745#file1579745line1778>
> >
> >     The path of the JAR output directory can be specified with bindir Sqoop options so I can imagine there are some users who would like to have the JAR file after the Sqoop import/export has finished. Even if we use a temp directory I can imagine some people do not override the default output directory but still use the JAR file.
> 
> Eric Lin wrote:
>     This sounds like we need an option to ask sqoop whether to keep temp jar files or not. Based on current code, they will never be deleted and in most cases, users don't care about them.

New option added: --delete-compile-dir


> On Dec. 12, 2016, 8:57 a.m., Szabolcs Vasas wrote:
> > src/java/org/apache/sqoop/orm/CompilationManager.java
> > Lines 242 (patched)
> > <https://reviews.apache.org/r/54528/diff/1/?file=1579746#file1579746line242>
> >
> >     The destination directory path can be specified using -Dsqoop.src.dir option, so I can imagine some users rely on the availability of the generated source code after Sqoop finished importing/exporting.
> 
> Eric Lin wrote:
>     I will test this again, thanks

Added option "--delete-compile-dir", so user can control whether or not to clean up the compile directory.


- Eric


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


On Jan. 17, 2017, 7:24 a.m., Eric Lin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54528/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2017, 7:24 a.m.)
> 
> 
> Review request for Sqoop and Szabolcs Vasas.
> 
> 
> Bugs: SQOOP-3042
>     https://issues.apache.org/jira/browse/SQOOP-3042
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> After running sqoop, all the temp files generated by ClassWriter are left behind on disk, so anyone can check those JAVA files to see the schema of those tables that Sqoop has been interacting with. By default, the directory is under /tmp/sqoop-<username>/compile.
> 
> In class org.apache.sqoop.SqoopOptions, function getNonceJarDir(), I can see that we did add "deleteOnExit" on the temp dir:
>     for (int attempts = 0; attempts < MAX_DIR_CREATE_ATTEMPTS; attempts++) {
>       hashDir = new File(baseDir, RandomHash.generateMD5String());
>       while (hashDir.exists()) {
>         hashDir = new File(baseDir, RandomHash.generateMD5String());
>       }
> 
>       if (hashDir.mkdirs()) {
>         // We created the directory. Use it.
>         // If this directory is not actually filled with files, delete it
>         // when the JVM quits.
>         hashDir.deleteOnExit();
>         break;
>       }
>     }
> However, I believe it failed to delete due to directory is not empty.
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/SqoopOptions.java 30b4705 
>   src/java/org/apache/sqoop/orm/ClassWriter.java e289429 
>   src/java/org/apache/sqoop/orm/CompilationManager.java 0a2a87f 
> 
> 
> Diff: https://reviews.apache.org/r/54528/diff/1/
> 
> 
> Testing
> -------
> 
> I have tested manually. I have checked with a couple of other Java developers and it turned out that it is not easy to add test for deleteOnExit, so I did not add any test cases. The code path I changed does not seem to have test coverage either. Let me know if I am wrong.
> 
> Thanks
> 
> 
> Thanks,
> 
> Eric Lin
> 
>