jhtimmins commented on a change in pull request #16002:
URL: https://github.com/apache/airflow/pull/16002#discussion_r640233019
##########
File path: airflow/providers/microsoft/azure/log/wasb_task_handler.py
##########
@@ -176,19 +178,18 @@ def wasb_write(self, log: str, remote_log_location: str,
append: bool = True) ->
:type log: str
:param remote_log_location: the log's location in remote storage
:type remote_log_location: str (path)
- :param append: if False, any existing log file is overwritten. If True,
- the new log is appended to any existing logs.
+ :param append: if False and overwrite = True, any existing log file is
overwritten with the new log.
+ If True and overwrite = True, the new log is appended
to any existing logs.
+ If True and overwrite = False the method will fail
:type append: bool
+ :param overwrite: if False, does not overwrite log file. If the file
already exists the method will fail
+ if True, any existing log file is overwritten.
+ :type overwrite: bool
Review comment:
This behavior doesn't really make sense to me. Specifically, it seems
like `append + overwrite == append`.
More generally, I think that deriving behavior as the cross product of two
different arguments is an antipattern. Is there a way that this can be
simplified? Is `append=False && overwrite=False` a valid state? If not, and
there are only two valid. states, then this can be simplified into one argument.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
