-
Notifications
You must be signed in to change notification settings - Fork 11
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
Biothings transformer support for arbitrary complex queries #592
Comments
Note that there could be similar issues in other transformers, and further review is required to determine how this might interact with the JQ PRs. @rjawesome , do you have any insight? |
The JQ has access to this.edge, so a fix for this could be implemented by modifying the JQ transformers for biothings. |
I just pushed a fix for this on jq branch with a generateCurieWithInputs jq function that takes in the curie input so this can easily be used for any other APIs as well. @tokebe Just wanted to know, is the field "query" returned by biothings ever an array as currently my code doesn't handle that. |
I don't believe so, @colleenXu or @erikyao might be able to answer with more certainty. |
@rjawesome not sure if you mean "queryInputs" for "the curie input" mentioned in your post. I don't pay a lot of attention to what's in the "query" field...so I don't know. Maybe @erikyao or @newgene would know.. |
Also @tokebe I wonder if you could check @rjawesome's commit? biothings/api-respone-transform.js@ea9db14 I'm not sure that I'm understanding it correctly, but it seems to be doing something with prefixes and upper-case. This may not be relevant, but I'll note some things about prefixes, IDs, and letter-capitalization here
|
I just converted everything to uppercase when checking for inclusion so that casing wouldn't be an issue. Currently my code assumes queryInputs doesn't have a prefix but the case of a prefix could be handled by just splitting by ":". |
This is tied to the JQ work #489 and biothings/api-respone-transform.js#38 |
@tokebe Is it okay to close this issue? Isn't this only deployed on Dev/CI instances? (so we can't adjust the registered SmartAPI yamls if we want) |
This was auto-closed by commit-linking, whoops! |
Relevant changes deployed to Prod. |
Currently, the Biothings transformer gets input IDs from POST queries by passing the query associated with a hit into
generateCurie()
. This only supports queries formatted with the input ID at the very end of the string, asgenerateCurie()
splits a string by:
and takes the last item.Complex query strings can easily cause problems if they don't leave the ID at the end of the string. After discussion with @colleenXu, the most doable fix at the moment is to run through
queryInputs
(accessible in that code section viathis.edge
) and check for inclusion in the query string, taking the first item that matches, and then passing that togenerateCurie()
.The text was updated successfully, but these errors were encountered: