django-lot
django-lot copied to clipboard
jespino
The LOT model should use Django's model validation
There are two options here:
- Instead of https://github.com/jespino/django-lot/blob/master/lot/models.py#L42 we should use clean and raise a validation error if the token has expired.
- Call verify inside clean and raise a validation error in order to prevent creating/updating an expired/invalid token.
I prefer the second option because verify check that the token is valid in other way.
Use directly the clean methods have 2 problems, we use clean in an strange way, and we will use the Exception as a flow control structure.
Exceptions are flow control structures in python. MyModel.DoesNotExist is a very good example of how Pythonic code behaves.
Exceptions are for exceptional cases, and are less efficient than normal control structures.
That's true for most languages. Python uses an exception to stop a generator from iterating. See https://docs.python.org/2/library/exceptions.html#exceptions.StopIteration
Django uses ValidationError regularly in order to report validation failures. It's more pythonic.
Model validation is for ensuring the values in the fields are valid. The LOT.valid method is for checking if the token is valid for login.
These are not the same action, so I don't agree with 1
I also don't see any specific harm in creating an invalid token. It will get cleaned up in general usage, and there's no need to restrict a case that is harmless and potentially useful to someone.
Mmmm, I think one use case can be something like "you can directly login week days but no weekends" or other configuration not necesary valid on creation or modification but valid in the future.
Oh, yeah... a more flexible way to test validity... perhaps a callable in the config dict? Should I open a separate ticket? :)