EUDAT-B2STAGE / http-api

RESTful HTTP-API for the B2STAGE service inside the EUDAT project
https://eudat-b2stage.github.io/http-api/
MIT License
7 stars 7 forks source link

Catch rare exception during enable and return error to client. #139

Closed merretbuurman closed 5 years ago

merretbuurman commented 5 years ago

This PR addresses issue https://github.com/EUDAT-B2STAGE/http-api/issues/138

If the directory cannot be created, this is critical - the whole batch cannot be processed if this fails. So I return an error, but the problem definitely has to be solved on the server, not by the client.

TODO: The collection should definitely be deleted from irods, so that no success is returned when the client tries the same request a second time! I did not find a delete function in the rapydo irods client, that's why I did not do this yet.

merretbuurman commented 5 years ago

The last commit removes the TODO: Instead of deleting the irods collection in case of filesystem write failure, we create the irods collection after the attempt to write on the filesystem. So if the write fails, the collection is never created.

@mdantonio Please review this commit - maybe there was reasons for first creating the irods thing? Also now, the opposite problem occurs: If irods fails, we have a directory we don't need. So deleting the collection in the catch block might be better!

mdantonio commented 5 years ago

So deleting the collection in the catch block might be better!

Both creation should be catched and in case of errors reverted by deleting them

I did not find a delete function in the rapydo irods client, that's why I did not do this yet

imain.remove(batch_path, recursive=True, force=True)

Could you update the PR with the remove?

merretbuurman commented 5 years ago

Hi, I added the remove, will add a catch block also for the collection creation. Is IrodsException the correct exception to catch?

https://github.com/rapydo/http-api/blob/master/restapi/flask_ext/flask_irods/client.py#L19

merretbuurman commented 5 years ago

Functionality now:

merretbuurman commented 5 years ago

Note: Creating the superdirectory could be done directly in rapydo's path.create(), if the arg parents was passed through.

PR on rapydo for that: https://github.com/rapydo/utils/pull/19

See: https://docs.python.org/3/library/pathlib.html#pathlib.Path.mkdir.

mdantonio commented 5 years ago

Hi, I added the remove, will add a catch block also for the collection creation. Is IrodsException the correct exception to catch?

https://github.com/rapydo/http-api/blob/master/restapi/flask_ext/flask_irods/client.py#L19

Yes, it is