guan404ming commented on code in PR #62321:
URL: https://github.com/apache/airflow/pull/62321#discussion_r2854096040
##########
providers/imap/src/airflow/providers/imap/hooks/imap.py:
##########
@@ -310,11 +317,21 @@ def _correct_path(self, name: str,
local_output_directory: str) -> str:
else local_output_directory + "/" + name
)
- def _create_file(self, name: str, payload: Any, local_output_directory:
str) -> None:
- file_path = self._correct_path(name, local_output_directory)
-
- with open(file_path, "wb") as file:
- file.write(payload)
+ def _create_file(
+ self, name: str, payload: Any, local_output_directory: str,
overwrite_file: bool
+ ) -> None:
+ filename, extensions = name.split(".", 1)
Review Comment:
This will likely raise ValueError if the filename has no extension (e.g.,
"Makefile", "README"). We could consider to use a safer split like
`os.path.splitext` or handle the no-dot case. We could also add more test cases
to ensure we handle all cases.
##########
providers/imap/src/airflow/providers/imap/hooks/imap.py:
##########
@@ -310,11 +317,21 @@ def _correct_path(self, name: str,
local_output_directory: str) -> str:
else local_output_directory + "/" + name
)
- def _create_file(self, name: str, payload: Any, local_output_directory:
str) -> None:
- file_path = self._correct_path(name, local_output_directory)
-
- with open(file_path, "wb") as file:
- file.write(payload)
+ def _create_file(
+ self, name: str, payload: Any, local_output_directory: str,
overwrite_file: bool
+ ) -> None:
+ filename, extensions = name.split(".", 1)
+ counter = 1
+ method = "wb" if overwrite_file else "xb"
+ while True:
Review Comment:
This while-loop seems redundant here. The "wb" mode will never raise
FileExistsError, so the loop always runs exactly once, but it still splits the
filename and initializes a counter for no reason. Consider an early return for
the overwrite case to keep it clean:
```py
with open(file_path, "wb") as file:
file.write(payload)
return
```
Feel free to correct me if I'm wrong, thanks!
--
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]
