Question: Why not raise ValidationFailure?

This is an interesting little library and very useful (Thanks!). However, the validators.utils.validator decorator ends up making a weird little API since it returns the Exception instead of raising it which is further complicated by the fact that on success True is returned. Thus the code reads as:

x = validators.domain('/howdy$com')
if x is not True:
   raise MyError("Invalid Domain") from x

instead of the more normal:

try:
    validators.domain('/howdy$com')
except validators.ValidationError as ex:
    raise MyError("InvalidDomain") from ex

I'm primarily curious as to the reasoning behind this as it's simply not expected.

A few suggestions:

Instead of True return None so that at the very least the logic above can be simpler:

x = validators.domain('/howdy$com')
if x:
   raise X

Raise the exception instead of returning it, or provide a method to configure the library to do so.

>>> validators.raise_exceptions = True
>>> validators.domain('/howdy$com')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
validators.utils.ValidationFailure: ValidationFailure(func=domain, args={'value': '/howdy$com'})

or

>>> validators.domain('/howdy$com', raise_exceptions=True)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
validators.utils.ValidationFailure: ValidationFailure(func=domain, args={'value': '/howdy$com'})

NOTE: I'm suggesting a method of config to maintain backwards compatibility with the uses already out there.

I'm fine with having a configuration option for that. The reason for having an API like it currently has is that many times I found it cumbersome of catching exceptions. Let's say I only want to check that given value is a valid email, otherwise I'll ignore it. I can just write:

if validators.email(value):
   ... do something ...

instead of

try:
    validators.email(value)
except validators.ValidationError as ex:
    pass
else:
   ... do something ...

I was thinking more about this, and I think you can do it easily as a parameter:

try:
    validators.email(value, raise_on_error=True)
except validators.ValidationError:
    log.exception('error in validation')
    # do something about it
else:
   # continue on
   pass

This would just require the https://github.com/kvesteri/validators/blob/master/validators/utils.py#L82 to check for the kwarg and remove it from the dict passed onto the wrapped function; something like:

raise_on_error = kwargs.get('raise_on_error', False)
if 'raise_on_error' in kwargs:
   del kwargs['raise_on_error']
value = func(...)
if not value:
   value = ValidationError(...)
   if raise_on_error:
      raise value
  return value
else:
  return True

From your example, I'm thinking the above might be your only good solution to maintaining backwards compatibility which will be important; however, do realize that your logic may fail unless you return a False-y value in some form (None, False, 0, etc). At it is you're always returning the equivalent of True even when validation fails.

That is:

x = validators.email(...)
if x:
   print('Validation Succeeded!')

the above will always print out "Validation Succeeded!" whether ValidationError is returned or True is returned as they're both equivalent to True. If you returned None instead of True, then you could do:

x = validators.email(...)
if not x:
   print("Validation Succeeded!")

this would also remove the need to definitively tests using isinstance() in your unit tests.

As someone who loves to use this library, I like the raise_on_error flag idea!

A configuration would be great. Most times, people need True or False.

Navigation Menu

Repository navigation

Question: Why not raise ValidationFailure? #139

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Question: Why not raise ValidationFailure? #139

Description

Activity

kvesteri commented on Feb 14, 2020

BenjamenMeyer commented on Feb 18, 2020

InnovativeInventor commented on Feb 19, 2020

ogencoglu commented on Sep 4, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions