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

support jq based response transformation in SmartAPI annotations #521

Open
newgene opened this issue Oct 25, 2022 · 7 comments
Open

support jq based response transformation in SmartAPI annotations #521

newgene opened this issue Oct 25, 2022 · 7 comments
Assignees

Comments

@newgene
Copy link
Member

newgene commented Oct 25, 2022

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
Copy link
Collaborator

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
Copy link
Collaborator

colleenXu commented Oct 17, 2023

We've now decided that we're going to separate the "basic JQ in BTE" support #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 #489 that's relevant to this issue.

@colleenXu
Copy link
Collaborator

colleenXu commented Oct 17, 2023

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

@colleenXu
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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):
    • 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):
    • 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
Copy link
Collaborator

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...

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

3 participants