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.

Sign up for GitHub

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

Jump to bottom

Validator treats top-level 'included' array as mandatory #393

Closed
merkys opened this issue Jul 10, 2020 · 2 comments · Fixed by #395
Closed

Validator treats top-level 'included' array as mandatory #393

merkys opened this issue Jul 10, 2020 · 2 comments · Fixed by #395
Labels
validator Related to the OPTIMADE validator

Comments

@merkys
Copy link
Member

merkys commented Jul 10, 2020

I am trying to validate COD implementation (in development) using optimade-python-tools v0.9.8. The validator seems to insist on the existence of top-level included array, which is clearly optional, according to OPTIMADE specification:

$ optimade_validator https://www.crystallography.net/cod-test/optimade -v 5
Testing entire implementation at https://www.crystallography.net/cod-test/optimade...

Mandatory tests:
[snip]
2020-07-10 14:51:48,137 - optimade.validator.validator |    DEBUG: Testing multiple entry endpoint of references
✔: https://www.crystallography.net/cod-test/optimade/references?page_limit=5 - request successful.
✖: https://www.crystallography.net/cod-test/optimade/references?page_limit=5 - deserialize_response - failed with error
	ValidationError: 3 validation errors for ValidatorReferenceResponseMany
	included
	  field required (type=value_error.missing)
[snip]
@merkys merkys added the validator Related to the OPTIMADE validator label Jul 10, 2020
@ml-evs
Copy link
Member

ml-evs commented Jul 10, 2020

Thanks for this @merkys, looks like the problem is:

optimade-python-tools/optimade/validator/validator_model_patches.py

Line 36 in bf092ea

included: Optional[List[Dict[str, Any]]] = Field(...)

which wasn't updated properly when pydantic switched to Field from Schema (although included is marked as optional, unless we give it a a default value of None it will still expect a value). I'll make a PR for this right away.

ml-evs added a commit that referenced this issue Jul 10, 2020
@ml-evs
Give validator patched included a default value to make it really o…
22bb1b1 
…ptional (closes #393)
@ml-evs ml-evs mentioned this issue Jul 10, 2020
Merged
ml-evs added a commit that referenced this issue Jul 10, 2020
@ml-evs
Give validator patched included a default value to make it really o…
f33ef2e 
…ptional (closes #393)
ml-evs added a commit that referenced this issue Jul 10, 2020
@ml-evs
Give validator patched included a default value to make it really o…
cf50cf3 
…ptional (closes #393)
ml-evs added a commit that referenced this issue Jul 16, 2020
@ml-evs
Give validator patched included a default value to make it really o…
67eebf5 
…ptional (closes #393)
ml-evs added a commit that referenced this issue Jul 17, 2020
@ml-evs @CasperWA @merkys
Fix some validator-specific crashes (#395)
93e50b1 
* Give validator patched `included` a default value to make it really optional (closes #393)

* Validate relationships data as a list before testing type (closes #397)

* Improve --verbosity help string for validator (closes #396)

* Add concept of InternalErrors to validator

These errors should be handled differently to ValidationError and
ResponseError as they indicate a problem with the validator itself.

* Added validator flags --fail_fast and --skip_optional_tests

* Improvements to pagination validation

- Allow links objects to be passed in pagination
- Enforced maximum recursion depth (5) for pagination tests

* Reduce set of example queries from the specification, pending future improvements (#357)

* Improve error message for single entry endpoint if deserialization fails

* Implemented suggestions from code review

Co-authored-by: Casper Welzel Andersen <CasperWA@users.noreply.github.com>

* Renamed entrypoint optimade_validator->optimade-validator

* Flip arg options the "standard" way around

Use -t for --as-type (instead of -a).

Co-authored-by: Casper Welzel Andersen <casper.andersen@epfl.ch>
Co-authored-by: Andrius Merkys <andrius.merkys@gmail.com>
@merkys
Copy link
Member Author

merkys commented Jul 22, 2020

I confirm this is fixed as of v0.10.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
validator Related to the OPTIMADE validator
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix some validator-specific crashes Materials-Consortia/optimade-python-tools
2 participants
@ml-evs @merkys