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

Dont run validates_schema if individual fields were invalid #323

Closed
jjvattamattom opened this Issue Nov 5, 2015 · 5 comments

Comments

Projects
None yet
5 participants
@jjvattamattom

jjvattamattom commented Nov 5, 2015

I just discovered that validates_schema runs if individual fields have failed validation. I kinda expected otherwise.

I want a way to prevent validates_schema from being run if individual fields have failed validation. Please confirm if my way of comparing original and processed data is sufficient or that there is a simpler alternative.

@validates_schema(pass_original=True)
def validate_schema(self, data, original):
    # dont run below tests if validation on individual fields failed:
    if sorted(data.keys()) != sorted(original.keys()):
        return
    ...tests that presume individual fields are valid
@sloria

This comment has been minimized.

Member

sloria commented Nov 11, 2015

Sorry for the delayed response. I'm not sure there is a clean way to achieve this at the moment. I don't think we should change the default behavior; there are use cases where you want schema validators to run even if field validators fail. That said, I am open to suggestions for supporting the OP's use case.

@DamianHeard

This comment has been minimized.

Contributor

DamianHeard commented Nov 26, 2015

Perhaps an alternative would be to properly expose the errors that have been captured during unmarshalling. At the moment you have to go through self._unmarshal.errors. Individual validate_schema methods would either check for errors before running of the decorator could be modified to automatically do this check if configured. Of course, I could be missing something and these errors might be already getting exposed at a more desirable location.

@d-sutherland

This comment has been minimized.

Contributor

d-sutherland commented Nov 30, 2015

Another simple alternative would be to provide this as an argument to the validation decorators, allowing full backwards compatibility.
Yet another alternative is to call validates_schema methods when a field has already failed only if strict mode is off. As such when strict mode is on the errors are propagated immediately (or at least it appears so to the user), which follows the same pattern users would be familiar with for exceptions. This would also follow the principle of least astonishment - if an error is raised (as occurs when strict is True) then the user can expect that code further down the pipeline won't be reached.

@maximkulkin

This comment has been minimized.

Contributor

maximkulkin commented Nov 30, 2015

I just hit the same issue which got me surprised. IMO the most sane behavior is to:

  • Deserialize fields and let field validations to run
  • For each field run validations declared with validates(field_name) decorator only if that field does not have errors
  • If there are no errors, run validations declared with validates_schema decorator.

d-sutherland added a commit to d-sutherland/marshmallow that referenced this issue Dec 1, 2015

Optional skip_on_field_errors arg for validates_schema
Add 'skip_on_field_errors' parameter to validates_schema.
'skip_on_field_errors' allows schema level validation to be skipped
when errors are detected in field validation removing the need for
the user to check.
Address issue marshmallow-code#323

d-sutherland added a commit to d-sutherland/marshmallow that referenced this issue Dec 2, 2015

Updates to optional skip_on_field_errors arg
Add testing for validate kwarg of fields and make
field_errors argument to invoke_validators optional for
backwards compatibility.
Address issue marshmallow-code#323
@sloria

This comment has been minimized.

Member

sloria commented Dec 8, 2015

The skip_on_field_errors parameter has been added to validates_schema in 2.4.0.

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