kbaseattic / relation_engine_spec

Specifications and config for the KBase Relation Engine
https://kbase.us
MIT License
0 stars 7 forks source link

Reorganize queries #67

Closed jayrbolton closed 5 years ago

jayrbolton commented 5 years ago
jayrbolton commented 5 years ago

@MrCreosote I added AQL syntax checking, plus bind var validation using the arangodb query parsing endpoint. Turned out to be pretty easy and uncovered a couple bugs

Note on the @ws_ids bind var: this is a special bind var that is set by the RE API for every query of all workspaces the requester has access to. In AQL, if you have a bind var set, but it doesn't appear in the query, then an error gets thrown. However, we don't want to use this variable in all queries (eg. taxonomy). So the RE API will prepend the clause LET ws_ids = @ws_ids to prevent the error. To be consistent, we can just use ws_ids when we need to use that var.

MrCreosote commented 5 years ago

this is ready for re-review then?

jayrbolton commented 5 years ago

Ready for re review

On Mon, Jul 22, 2019 at 5:41 PM MrCreosote notifications@github.com wrote:

this is ready for re-review then?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/kbase/relation_engine_spec/pull/67?email_source=notifications&email_token=ACCV47FIPR3XHWIOKJOZTEDQAZHVBA5CNFSM4IF5XFP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2RSIDQ#issuecomment-514008078, or mute the thread https://github.com/notifications/unsubscribe-auth/ACCV47DZU5W6MUR7E7JTBB3QAZHVBANCNFSM4IF5XFPQ .

jayrbolton commented 5 years ago

@MrCreosote I made some more updates, ready for re-review if you have time:

For json schema default values, I'm going to add something like this in the RE API: https://python-jsonschema.readthedocs.io/en/stable/faq/

MrCreosote commented 5 years ago

Looks ok to me other than the question about the cached fn call. I didn't look at the stored queries though.