biothings / biothings_explorer

TRAPI service for BioThings Explorer
https://explorer.biothings.io
Apache License 2.0
10 stars 11 forks source link

support jq based response transformation in SmartAPI annotations #521

Open newgene opened 2 years ago

newgene commented 2 years ago

In issue #489, @rjawesome evaluated both JMESPath and jq, then decided to use jq as the JSON transformation engine for it richer functionality.

This issue continues the implementation in PR38 for issue #489 to provide an option for users to include jq based response transformation directly in an API's SmartAPI annotations. The generic JQTransformer will take the provided jq string and performance the transformation.

colleenXu commented 1 year ago

Based on yesterday's meetings, we are working towards this issue and the "general JQ support" together. The api-response-transform PR seems to include support for both issues, while the smartapi-kg PR seems specific to this issue.

The main discussions with likely happen in #489.

Our plan is to move external APIs (non-BioThings, non-TRAPI, or not made by our lab) to having JQ-support in the SmartAPI yamls. See this post for more details

colleenXu commented 1 year ago

We've now decided that we're going to separate the "basic JQ in BTE" support https://github.com/biothings/biothings_explorer/issues/489 from this issue (ref: Jackson's post and Chunlei's reply), because there may be more refactoring needed to make this issue easier to use.

I'm linking some discussion from https://github.com/biothings/biothings_explorer/issues/489 that's relevant to this issue.

colleenXu commented 1 year ago

However, we have worked on "JQ in SmartAPI". Here's where we are:

colleenXu commented 1 year ago

And a piece of feedback I have: the field-name in x-bte annotation is currently transformer. Is that fine, or would transformers be better?

colleenXu commented 1 year ago

The smartapi-kg PR is tied to this issue.

And there's some thoughts on how code for this issue would be deployed in the future in the other issue, and I'll paste them here:

Jackson: "smartapi-kg PR likely won't need significant changes in the future (everything needing changes that I'm aware of for future jq-in-smartapi stuff is in the api-response-transform)"

Me: if we waited to deploy the smartapi-kg PR with the future api-response-transform PR, we can also deploy the updated yamls right away - instances that don't have the PRs yet won't be able to ingest the new fields and will just ignore it

colleenXu commented 1 year ago

And some of my notes on how to test "JQ in SmartAPI yamls / x-bte annotation". Not sure if it'll be relevant once we are ready to proceed

click to expand

Test "JQ in SmartAPI" Process: * For each API that we want to "move JQ into SmartAPI yamls / x-bte annotation"...(was previously written [here](https://github.com/biothings/biothings_explorer/issues/489#issuecomment-1718298042)): * check out the correct branches * Main testing that BTE is using "jq in smartapi" and it's working: * Remove "jq in bte", add "jq in smartapi" in local files * use smartapi override for local files, test that the operations work * double-check that having "jq in bte" doesn't cause issues: Put "jq in bte" back, do a quick test that it still works * At the end, do edge-case test (from the first square bullet [here](https://github.com/biothings/biothings_explorer/issues/489#issuecomment-1747579728)): * Check out main branches * Use the smartapi yamls with "jq in smartapi" * Do a quick test with each API, testing that BTE is doing fine. It should be ignoring the "JQ related fields" for operations… Note: CTD uses a "pair" string, which is a way to test that it works

colleenXu commented 1 year ago

and a note from our discussion of x-bte refactoring:

JQ in SmartAPI is maybe for "basic post-processing" only. And more "mental model" or complex stuff should stay in BTE (I'm not sure what "mental model" means here now...).

Since we want to avoid making the x-bte annotation writing harder/more-complicated and more like code...