sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Eric Lin via Review Board <nore...@reviews.apache.org>
Subject Re: Review Request 54528: SQOOP-3042 - Sqoop does not clear compile directory under /tmp/sqoop-<username>/compile automatically
Date Wed, 11 Jul 2018 08:11:48 GMT


> On July 9, 2018, 11:34 a.m., Szabolcs Vasas wrote:
> > .gitignore
> > Lines 30 (patched)
> > <https://reviews.apache.org/r/54528/diff/3/?file=2052336#file2052336line30>
> >
> >     Why do we need this new folder here?

It happened to me a few times that "out" directory was created after running "ant jar-all"
or "ant test" commands, as those are the ones I run most often. However, not every time though,
so I am not sure what exactly will create this directory, but definitely happened to me several
times. (I tested again, it was not created anymore)


> On July 9, 2018, 11:34 a.m., Szabolcs Vasas wrote:
> > src/java/org/apache/sqoop/orm/ClassWriter.java
> > Lines 1793 (patched)
> > <https://reviews.apache.org/r/54528/diff/3/?file=2052338#file2052338line1793>
> >
> >     I think this shutdown hook should not be defined as an anonymous inner class
but should be extracted in a separate class.
> >     You can also consider using org.apache.commons.io.FileUtils#deleteDirectory
method instead of reimplementing the same.

Fixed


- Eric


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


On July 5, 2018, 8:03 a.m., Eric Lin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54528/
> -----------------------------------------------------------
> 
> (Updated July 5, 2018, 8:03 a.m.)
> 
> 
> Review request for Sqoop, Boglarka Egyed 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
> -----
> 
>   .gitignore 68cbe287 
>   src/java/org/apache/sqoop/SqoopOptions.java 3a19aeac 
>   src/java/org/apache/sqoop/orm/ClassWriter.java a4a768af 
>   src/java/org/apache/sqoop/orm/CompilationManager.java 6590cacc 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 8d318327 
> 
> 
> Diff: https://reviews.apache.org/r/54528/diff/3/
> 
> 
> 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
> 
>


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