Handle file name error in DELETE API route

[daniil-berg]

Requesting DELETE from the /api/file/<hash><ext> endpoint currently provides no feedback, as to whether the storage node even controls a file with the provided hash/extension at all. If it does not, a warning is logged, but the client receives the same 200 response either way.

videbo/src/videbo/storage/file_controller.py

Lines 410 to 417 in e7b53e0

	file = self.get(file_hash)
	if file is None or file.ext != file_ext:
	log.warning(f"So such file to remove: {file_hash}{file_ext}")
	return
	log.info(f"Scheduling file deletion: {file_hash}{file_ext}")
	TaskManager.fire_and_forget(
	self.remove_files(file_hash, origin=origin)
	)

No feedback/guarantees about successful deletion makes sense, but if the client application provided an invalid/unknown hash, it would be polite to respond accordingly. In this case, a 404 seems appropriate. The client is still free to ignore such a status code, but it may also be valuable information, in case there is a bug on the other end.

@larsbonczek What do you think? The way I see it, there are no implications for mod_videoservice because it already ignores any non-200 response to a deletion request anyway. If anything, this is an opportunity to log this specific 404 response on that end, too. That way we could more easily identify problems in setups with multiple LMS connected to one Videbo server. We always want to make sure that a file is only deleted, if it is no longer needed on any registered LMS. Better logging in the deletion process just seems like a good idea. (That is a topic for mod_videoservice, I just wanted to share my motivation for this issue here.)

[larsbonczek]

I agree that it makes sense to return a 404 status code if the specified file was not found. Although it would probably be even more interesting to receive a 500 status code if the deletion fails, because in that case the LMS may want to try again or log an error.

[daniil-berg]

@larsbonczek The problem with returning a deletion confirmation is that there is more than one reason for a file not being deleted. One is that some error occurred, but another is that it is still in use by another LMS connected to the same Videbo server. And while a 500 may be appropriate for the former, it certainly would not be for the latter.

Moreover, since the storage node will first check with every registered LMS and ensure none of them needs the file anymore before attempting deletion, the entire process from a DELETE request by one LMS to (un-)successful file deletion can take a significant amount of time. We decided to avoid a potentially long wait time on the client side by simply scheduling that entire process in the background. The downside is that the client receives no notice about whether or not the file was actually deleted.

Even if we chose to wait, it is unclear, if we really should inform the client one way or another. The mere info that yes, the file was found, but no, we chose to not remove it, already tells the client that it is probably in use on another LMS. This could become a privacy issue. After all, the client should not concern itself with other clients.

Lastly, even if something goes wrong and the file is not removed, this is not the client's business. It did everything correctly. It is up to the server to clean up the mess. And in fact, this is already handled by the option to search for orphan files. While this is not currently done automatically/periodically, this does make it possible to clean up any "leftovers" from time to time.

In conclusion, I would add a 404 response to the endpoint when the file is not found, but otherwise keep it minimal.