stoty commented on code in PR #1808:
URL: https://github.com/apache/phoenix/pull/1808#discussion_r1470804164


##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java:
##########
@@ -1557,7 +1558,7 @@ public void 
testCoprocessorsForCreateIndexOnOldImplementation() throws Exception
             // Now roll back to the old indexing
             IndexUpgradeTool iut =
                     new IndexUpgradeTool(ROLLBACK_OP, tableName, null,
-                            "/tmp/index_upgrade_" + 
UUID.randomUUID().toString(), false, null,
+                            filePath + "/index_upgrade_" + 
UUID.randomUUID().toString(), false, null,

Review Comment:
   In the Hadoop Tools, path parameters are some times HDFS paths, so that is a 
case where simple "/tmp" is the right solution.
   Those are also separate HDFS file systems, so there is no danger of 
coliision.
   
   Have you checked that all of these instances are used as local paths, as 
opposed to HadoopFS paths ?
   
   (I have checked this one, and this is correct, as this is a local FS path)



##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java:
##########
@@ -89,6 +89,7 @@
 
 @Category(ParallelStatsDisabledTest.class)
 public class CreateTableIT extends ParallelStatsDisabledIT {
+    private static final String filePath = 
System.getProperty("java.io.tmpdir");

Review Comment:
   This name is not descriptive.
   tmpPath would be better (in each file)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to