[GitHub] [airflow] XD-DENG commented on a diff in pull request #26886: Adding `preserve_file_name` param to `S3Hook.download_file` method

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

XD-DENG commented on code in PR #26886:
URL: https://github.com/apache/airflow/pull/26886#discussion_r988889356


##########
airflow/providers/amazon/aws/hooks/s3.py:
##########
@@ -902,14 +911,21 @@ def download_file(self, key: str, bucket_name: str | None = None, local_path: st
             else:
                 raise e
 
-        with NamedTemporaryFile(dir=local_path, prefix='airflow_tmp_', delete=False) as local_tmp_file:
+        if preserve_file_name:
+            local_dir = local_path if local_path else gettempdir()

Review Comment:
   To follow up on what @uranusjr 's thought:
   
   Say we are using LocalExecutor or CeleryExecutor, so two users' jobs can be executed on the same host.
   
   Here you are having `filename_in_s3 = s3_obj.key.rsplit('/', 1)[-1]`. So if user A is having file `.../A/data.json` and user B is having `.../B/data.json`, there may be conflict, right?
   
   But just a vague thinking and very likely I missed something. Please feel free to point out.



-- 
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