henry3260 commented on code in PR #60398:
URL: https://github.com/apache/airflow/pull/60398#discussion_r2681650623
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/s3.py:
##########
@@ -1743,29 +1743,29 @@ def _sync_to_local_dir_delete_stale_local_files(self,
current_s3_objects: list[P
def _sync_to_local_dir_if_changed(self, s3_bucket, s3_object,
local_target_path: Path):
should_download = False
- download_msg = ""
+ download_reasons = []
if not local_target_path.exists():
should_download = True
- download_msg = f"Local file {local_target_path} does not exist."
+ download_reasons.append(f"Local file {local_target_path} does not
exist.")
Review Comment:
> Instead of using f-string, can we do something like this instead?
>
> ```python
> download_logs = []
> download_log_params = []
> if ...
> download_logs.append("Local file %s does not exist.")
> download_log_params.append(local_target_path)
> ...
> download_logs.append("Downloaded %s to %s")
> download_log_params.extend((s3_object.key, local_target_path.as_posix()))
> self.log.debug(" ".join(download_logs), *download_log_params)
> ```
>
> Or, maybe just log each message separately. There’s no need to combine
them into one line.
Applied!
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
