Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preconditionfailed vs aborted #3003

Merged
merged 7 commits into from
Jun 23, 2022

Conversation

butonic
Copy link
Contributor

@butonic butonic commented Jun 23, 2022

Webdav distinguishes between 412 precondition failed for if match errors for locks or etags, uses 405 Method Not Allowed when trying to MKCOL an already existing collection and 409 Conflict when intermediate collections are missing.

The CS3 GRPC status codes are modeled after https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto. When trying to use the error codes to distinguish these cases on a storageprovider CreateDir call we can map ALREADY_EXISTS to 405, FAILED_PRECONDITION to 409 and ABORTED to 412.

Unfortunately, we currently use and map FAILED_PRECONDITION to 412. I assume because the naming is very similar to PreconditionFailed. However the GRPC docs are very clear that ABORTED should be used, specifically mentioning etags and locks.

With this PR we internally clean up the usage in the decompesedfs and mapping in the ocdav handler.

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@butonic butonic requested review from a team, labkode, ishank011 and glpatcern as code owners June 23, 2022 11:51
@butonic butonic marked this pull request as draft June 23, 2022 11:51
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@butonic butonic force-pushed the preconditionfailed-vs-aborted branch 2 times, most recently from 2dac9d1 to 9fda17b Compare June 23, 2022 13:59
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@butonic butonic marked this pull request as ready for review June 23, 2022 15:30
Copy link
Contributor

@C0rby C0rby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. 👍

@butonic butonic merged commit 1f3daf9 into cs3org:edge Jun 23, 2022
@butonic butonic deleted the preconditionfailed-vs-aborted branch June 23, 2022 15:36
wkloucek added a commit that referenced this pull request Aug 18, 2022
* revert some locking status codes back to precondition failed, see also #3003 and owncloud/ocis#4366

* fix unit tests

* fix locking response codes in ocdav

* fix refresh lock unit test
@kobergj kobergj mentioned this pull request Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants