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

Validate mandatory query field structure_features #678

Closed
CasperWA opened this issue Jan 18, 2021 · 3 comments · Fixed by #676
Closed

Validate mandatory query field structure_features #678

CasperWA opened this issue Jan 18, 2021 · 3 comments · Fixed by #676
Labels
enhancement New feature or request priority/medium Issue or PR with a consensus of medium priority validator Related to the OPTIMADE validator

Comments

@CasperWA
Copy link
Member

CasperWA commented Jan 18, 2021

I would argue they all should still fail due to them not supporting the mandatory query on structure_features.

Originally posted by @CasperWA in #676 (comment)

The validator should fail an implementation if querying structure_features fails, since it is mandatory to support queries for this field (in the /structures endpoints).

@ml-evs
Copy link
Member

ml-evs commented Jan 18, 2021

So the reason we're not validating this at the moment is that the validator can never find any values to query for with structure features, as the list is empty for almost everyone. This might be an easy fix to replace an if test_value with an if test_value is None as the default value will always be [] for structure_features.

@CasperWA
Copy link
Member Author

Why not just test for a value we know could be present (from the models)? The point is more that the implementation should at least return an empty array if there are no structures with the chosen value. I.e., they shouldn't fail with errors or similar.

@ml-evs
Copy link
Member

ml-evs commented Jan 18, 2021

Yup, that's what I'm trying to do now, just needs some extras added to the config.

@ml-evs ml-evs added bug Something isn't working validator Related to the OPTIMADE validator priority/medium Issue or PR with a consensus of medium priority enhancement New feature or request and removed bug Something isn't working labels Jan 18, 2021
ml-evs added a commit that referenced this issue Jan 18, 2021
ml-evs added a commit that referenced this issue Jan 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority/medium Issue or PR with a consensus of medium priority validator Related to the OPTIMADE validator
Projects
None yet
2 participants