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

(Updated July 11, 2018, 8:13 a.m.)


Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas.


Changes
-------

Addressed Szabolcs's feedback. Will check Boglarka's feedback separately


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 (updated)
-----

  .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 
  src/java/org/apache/sqoop/util/DirCleanupHook.java PRE-CREATION 


Diff: https://reviews.apache.org/r/54528/diff/4/

Changes: https://reviews.apache.org/r/54528/diff/3-4/


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

Reply via email to