[GitHub] [dolphinscheduler] caishunfeng commented on a diff in pull request #12486: [Bug] Set tenantDir permission

[GitBox <gi...@apache.org>]

caishunfeng commented on code in PR #12486:
URL: https://github.com/apache/dolphinscheduler/pull/12486#discussion_r1002493661


##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -85,26 +85,36 @@ public static String getUploadFilename(String tenantCode, String filename) {
         return fileName;
     }
 
+    public static String getTenantDir(String tenant) {
+        return DATA_BASEDIR + "/exec/process/" + tenant;

Review Comment:
   ```suggestion
           return DATA_BASEDIR + EXEC_PROCESS + tenant;
   ```



##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -85,26 +85,36 @@ public static String getUploadFilename(String tenantCode, String filename) {
         return fileName;
     }
 
+    public static String getTenantDir(String tenant) {
+        return DATA_BASEDIR + "/exec/process/" + tenant;
+    }
+
     /**
      * directory of process execution
      *
-     * @param projectCode project code
-     * @param processDefineCode process definition Code
+     * @param tenant               tenant
+     * @param projectCode          project code
+     * @param processDefineCode    process definition Code
      * @param processDefineVersion process definition version
-     * @param processInstanceId process instance id
-     * @param taskInstanceId task instance id
+     * @param processInstanceId    process instance id
+     * @param taskInstanceId       task instance id
      * @return directory of process execution
      */
-    public static String getProcessExecDir(long projectCode, long processDefineCode, int processDefineVersion,
-                                           int processInstanceId, int taskInstanceId) {
-        String fileName = String.format("%s/exec/process/%d/%s/%d/%d", DATA_BASEDIR,
-                projectCode, processDefineCode + "_" + processDefineVersion, processInstanceId, taskInstanceId);
-        File file = new File(fileName);
-        if (!file.getParentFile().exists()) {
-            file.getParentFile().mkdirs();
-        }
-
-        return fileName;
+    public static String getProcessExecDir(String tenant,
+                                           long projectCode,
+                                           long processDefineCode,
+                                           int processDefineVersion,
+                                           int processInstanceId,
+                                           int taskInstanceId) {
+        return String.format(
+                "%s/exec/process/%s/%d/%d_%d/%d/%d",
+                DATA_BASEDIR,
+                tenant,
+                projectCode,
+                processDefineCode,
+                processDefineVersion,
+                processInstanceId,
+                taskInstanceId);

Review Comment:
   ```suggestion
           return String.format(
                   "%s/%s/%s/%d/%d_%d/%d/%d",
                   DATA_BASEDIR,
                   EXEC_PROCESS,
                   tenant,
                   projectCode,
                   processDefineCode,
                   processDefineVersion,
                   processInstanceId,
                   taskInstanceId);
   ```



##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionCheckerUtils.java:
##########
@@ -131,4 +138,23 @@ public static void downloadResourcesIfNeeded(StorageOperate storageOperate,
             }
         }
     }
+
+    private static void createDirectoryWithOwner(Path filePath, String tenant) {
+        if (Files.exists(filePath)) {
+            return;
+        }
+        try {
+            Files.createDirectories(filePath);
+            if (!OSUtils.isSudoEnable()) {
+                // we need to open sudo, then we can change the owner.
+                return;

Review Comment:
   Is is better to throw a exception here?



##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionCheckerUtils.java:
##########
@@ -131,4 +138,23 @@ public static void downloadResourcesIfNeeded(StorageOperate storageOperate,
             }
         }
     }
+
+    private static void createDirectoryWithOwner(Path filePath, String tenant) {
+        if (Files.exists(filePath)) {
+            return;
+        }
+        try {
+            Files.createDirectories(filePath);
+            if (!OSUtils.isSudoEnable()) {
+                // we need to open sudo, then we can change the owner.
+                return;
+            }
+            UserPrincipalLookupService userPrincipalLookupService =
+                    FileSystems.getDefault().getUserPrincipalLookupService();
+            UserPrincipal tenantPrincipal = userPrincipalLookupService.lookupPrincipalByName(tenant);
+            Files.setOwner(filePath, tenantPrincipal);

Review Comment:
   Does it also work in mac or windows?



-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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