Skip to content
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

Consider allowing validation to be turned off #39

Open
bryanwweber opened this issue Mar 3, 2017 · 13 comments
Open

Consider allowing validation to be turned off #39

bryanwweber opened this issue Mar 3, 2017 · 13 comments
Assignees
Milestone

Comments

@bryanwweber
Copy link
Member

It would be very useful for quick & dirty work (particularly while the schema is changing so much) to be able to disable validation so that we can use PyKED without having to rewrite YAML files every week. Obviously all the files that go in the database have to validate properly, but this might be useful long term too.

@kyleniemeyer
Copy link
Member

Yeah, definitely. I added this in as a hack by doing return True at the top of _validate_isvalid_reference(), but would be good to add a method to do this.

@kyleniemeyer
Copy link
Member

kyleniemeyer commented Mar 8, 2017

Do we want to disable all validation, or just reference validation?

I suppose "all" validation just includes the methods in OurValidator: _validate_isvalid_unit(), _validate_isvalid_quantity(), _validate_isvalid_reference(), and _validate_isvalid_orcid().

@kyleniemeyer
Copy link
Member

Or, do we just want to have it skip all validation, as in just skip over self.validate_yaml(properties)?

@bryanwweber
Copy link
Member Author

I think all validation, but maybe we could make it customizable somehow

@kyleniemeyer
Copy link
Member

Yeah, that's a good idea. I can imagine use cases where you want to skip reference/author validation (which requires internet access and could be slow, etc.), but still ensure required fields are present.

I've started a branch to add this, and will finish it today.

@kyleniemeyer kyleniemeyer self-assigned this Mar 8, 2017
@bryanwweber
Copy link
Member Author

Watch out though, because a lot of the validation code will change with the uncertainty branch, so doing this now might make for tougher merges later 😃

@kyleniemeyer
Copy link
Member

Yeah yeah... I recognize that is coming from you wanting me to wrap up my uncertainty branch stuff 😋

Honestly though, I was planning on implementing all this with one or more variables like _skip_validation and just checking if that is true at the beginning of the custom validator functions... so we might need to add that in a place or two with uncertainty, but hopefully it won't be totally borked.

@bryanwweber
Copy link
Member Author

OK sounds good 😝

@bryanwweber bryanwweber added this to the v0.2 milestone Apr 2, 2017
@bryanwweber
Copy link
Member Author

See #49 for a partial implementation of this

@kyleniemeyer
Copy link
Member

I'm fine with closing this—there may be reasons to add the ability to disable parts of the validation with more granularity, but this should be sufficient for most needs.

@bryanwweber
Copy link
Member Author

I still think its worth having general schema validation without having DOI/ORCID checks. I actually think a more common use will be to turn off online validation but leave the rest of the validation turned on. I'm going to open this again so we remember to implement that, but it doesn't have to be high priority

@kyleniemeyer
Copy link
Member

kyleniemeyer commented Sep 26, 2017

OK, so I agree at minimum that we should be able to disable validation of the reference field—it should still be proper YAML, but have Cerberus ignore it. Perhaps in keeping with the existing option, the option could be skip_reference_validation=False (by default) ?

@kyleniemeyer
Copy link
Member

OK, so here's where we are now: reference with year and authors is required, but nothing else.

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

No branches or pull requests

2 participants