Return errors when deleting file (or files) fails?

[michallepicki]

Currently the Delete action just returns :ok, no matter what the adapter returned.

The local adapter would return an error tuple when that fails
And similarly the GCS adapter I'm using would also return an error tuple
The S3 adapter on the other hand discards the result and always returns :ok

I have a use case where I would like to check the result of file removal and act accordingly depending on the error. Would it be possible to expose results of that action? Would that be useful? I can work on a PR. This seems like a breaking change so maybe only when appropriate option is passed? I would also change the S3 adapter to just return what ExAws returned.

[achempion]

PR is welcome 👍

if it always returns :ok, then adding {:error, reason} in case it fails should not be a breaking change

[michallepicki]

@achempion I think the potential breaking change here is subtle because currently there is no contract between waffle's delete action and adapter code. So we can only hope that assuming :ok or {:ok, some_value} on success won't break the adapters.

What should be the return value if removing some of multiple versions fails? Something like {:error, [{version, returned_error}]}? Should it be a list when there are multiple versions, or when there are multiple versions and multiple errors?

[achempion]

{:error, [{version, returned_error}]}

good approach 👍

[achempion]

:ok on success shouldn't break as we currently return the same value, the error messages just would help reveal the deletion issues