[GitHub] [airflow] uranusjr commented on a diff in pull request #28477: Ensure correct log dir in file task handler

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

uranusjr commented on code in PR #28477:
URL: https://github.com/apache/airflow/pull/28477#discussion_r1055190200


##########
airflow/utils/log/file_task_handler.py:
##########
@@ -318,6 +319,34 @@ def read(self, task_instance, try_number=None, metadata=None):
 
         return logs, metadata_array
 
+    def _prepare_log_folder(self, directory: str):
+        """
+        To handle log writing when tasks are impersonated, the log files need to
+        be writable by the user that runs the Airflow command and the user
+        that is impersonated. This is mainly to handle corner cases with the
+        SubDagOperator. When the SubDagOperator is run, all of the operators
+        run under the impersonated user and create appropriate log files
+        as the impersonated user. However, if the user manually runs tasks
+        of the SubDagOperator through the UI, then the log files are created
+        by the user that runs the Airflow command. For example, the Airflow
+        run command may be run by the `airflow_sudoable` user, but the Airflow
+        tasks may be run by the `airflow` user. If the log files are not
+        writable by both users, then it's possible that re-running a task
+        via the UI (or vice versa) results in a permission error as the task
+        tries to write to a log file created by the other user.
+
+        Create the log file and give it group writable permissions
+        TODO(aoen): Make log dirs and logs globally readable for now since the SubDag
+        operator is not compatible with impersonation (e.g. if a Celery executor is used
+        for a SubDag operator and the SubDag operator has a different owner than the
+        parent DAG)
+        """
+        mode = 0o777
+        Path(directory).mkdir(mode=mode, parents=True, exist_ok=True)
+        current_mode = stat.S_IMODE(os.stat(directory).st_mode)
+        if current_mode != mode:
+            os.chmod(directory, mode)

Review Comment:
   If `directory` is a `Path`, this can simply be
   
   ```python
   directory.mkdir(mode=mode, parents=True, exist_ok=True)
   if directory.stat().st_mode != mode:
       directory.chmod(mode)
   ```



-- 
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@airflow.apache.org

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