Skip to content
This repository was archived by the owner on Jan 29, 2022. It is now read-only.

Make quota exceeded failures 400 errors in REST#145

Merged
zachmullen merged 2 commits intomasterfrom
quota-error-type
Feb 10, 2021
Merged

Make quota exceeded failures 400 errors in REST#145
zachmullen merged 2 commits intomasterfrom
quota-error-type

Conversation

@zachmullen
Copy link
Copy Markdown
Contributor

Fixes #144

@zachmullen zachmullen requested a review from brianhelba February 5, 2021 13:24
serializer.save(creator=user)
except QuotaLimitedError:
raise serializers.ValidationError(
{'size': ['This file would exceed the size quota for this folder.']}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If there's an easier way to achieve this response structure, let me know. I want it to mirror the structure of the other types of validation failure responses.

Copy link
Copy Markdown
Collaborator

@brianhelba brianhelba Feb 10, 2021

Choose a reason for hiding this comment

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

If the error occurs within a serializer field validator, the field will be automatically set. Since this is not thrown during validation, unfortunately it won't be auto-set here.

An alternative implementation could implement an internal API to determine whether a given allocation would exceed the quota. We'd then have to either do the entire API operation atomically, or we'd have to re-check the same thing on save anyway.

Since end users ultimately get the right response this way, and it seems simpler to implement (though slightly less "correct", since we want to try to do all validation in the serializer), I think the current way is best.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Actually, I'll weaken my position: I think the current way is adequate. The alternative would require a new File/Folder/Quota API, but probably won't be any more complex on the DRF layer, since we'd no longer catch exceptions during .save.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make quota exceeded failures on file creation a 400 rather than 500

2 participants