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

clarifiy / rename readonly-rule #127

Closed
funkyfuture opened this Issue Jul 13, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@funkyfuture
Copy link
Member

funkyfuture commented Jul 13, 2015

i think the readonly-rule isn't named intuitively. it could mean a lot and my guess is it was named for a certain use-case (propably validating before writing something). but it doesn't explain itself immediately and unambiguously. i had to read the code to fully get its purpose which turned out more trivial and explanatory to me than the docs.

i propose to rename it to is_exluded. excluded may be semantically enough, but it would be just one keystroke away from excludes. another proposal is blacklisted or to stick with an already used term: allowed.

the transition should be implemented like from key- to valueschema.

a usage-example should be added to the documentation.

or is this an artifact from the time before allow_unknown was available?

@funkyfuture

This comment has been minimized.

Copy link
Member

funkyfuture commented Jul 13, 2015

oh, allowed was a stupid idea.

on a second thought, it isn't. like allow_unknown it could be defined either by a bool or more specific with a list of allowed values (as it is now).

@funkyfuture

This comment has been minimized.

Copy link
Member

funkyfuture commented Mar 5, 2016

i pledge to resolve this before an 1.0 release.

@nicolaiarocci

This comment has been minimized.

Copy link
Member

nicolaiarocci commented Mar 6, 2016

I am not clear on what your suggestion is right now. In any case, as you mention, readonly should be deprecated, not removed entirely.

And yes the use case was receiving a payload for validation, before sending it to a permanent store. Sometime you have fields which should be there for reads, but should not be writable. From the perspective of a system like that, readonly actually makes some sense 😄 but I recon it might be confusing in different scenarios.

@funkyfuture

This comment has been minimized.

Copy link
Member

funkyfuture commented Mar 6, 2016

i have no clear suggestion. ;-) here are two:

a) clarifying the documentation. (also noting that it makes only sense if unknown fields are allowed.)

b) turn a {'field': {'readonly': True}} into {'field': {'allowed': False}}. this means that the allowed-rule with a boolean can be used to override the tolerant behaviour when allow_unknown is set for a specific field. since the field must not exist, it can't collide with a sequence of allowed values defined with allowed. it may be confusing anyway.

@nicolaiarocci

This comment has been minimized.

Copy link
Member

nicolaiarocci commented Mar 6, 2016

upgrading the docs should be fine I think.

@nicolaiarocci nicolaiarocci added this to the 1.0 milestone Mar 6, 2016

funkyfuture added a commit to funkyfuture/cerberus that referenced this issue Oct 26, 2016

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