updated to trapi 1.5 #236
updated to trapi 1.5 #236
Conversation
|
The failing tests, I believe are because there is not a strider/1.5 out there. |
|
OK, I modified the cache as discussed this morning. The one tricky thing is that overwrite_cache defaults to False so I think we lose e.g. caching of CQS queries. I also added logs:[] to created Responses (which happens when we make an infer response at merge time) |
src/service_aggregator.py
Outdated
| @@ -165,19 +174,19 @@ async def entry(message, guid, coalesce_type, caller) -> (dict, int): | |||
| # We told the world what we can do! | |||
| # Workflow will be a list of the functions, and the parameters if there are any | |||
|
|
|||
| read_from_cache = bypass_cache or overwrite_cache | |||
There was a problem hiding this comment.
The name of this variable is the reverse of what it does?
src/service_aggregator.py
Outdated
| results_cache.set_result(input_id, predicate, qualifiers, source_input, caller, workflow_def, final_answer) | ||
| elif {"id": "lookup"} in workflow_def or override_cache: | ||
| results_cache.set_lookup_result(workflow_def, query_graph, final_answer) | ||
| if overwrite_cache: |
There was a problem hiding this comment.
I think this should instead be if not bypass_cache: because we want to save everything to cache if it wasn't in there, unless users specifically say they want to bypass the whole thing.
| response = await subservice_post("strider", strider_url, message, guid, asyncquery=asyncquery, params=params) | ||
|
|
||
| return response | ||
|
|
||
|
|
||
| async def normalize_qgraph_ids(m): | ||
| url = f'{os.environ.get("NODENORM_URL", "https://nodenormalization-sri.renci.org/1.3/")}get_normalized_nodes' | ||
| url = f'{os.environ.get("NODENORM_URL", "https://nodenormalization-sri.renci.org/")}get_normalized_nodes' |
There was a problem hiding this comment.
Have we confirmed with Guarav to remove the trapi version?
src/service_aggregator.py
Outdated
| @@ -756,7 +830,7 @@ async def robokop_lookup(message, params, guid, infer, question_qnode, answer_qn | |||
| # For robokop, gotta normalize | |||
| message = await normalize_qgraph_ids(message) | |||
| if not infer: | |||
| kg_url = os.environ.get("ROBOKOPKG_URL", "https://automat.renci.org/robokopkg/1.4/") | |||
| kg_url = os.environ.get("ROBOKOPKG_URL", "https://automat.renci.org/robokopkg/1.5/") | |||
There was a problem hiding this comment.
Does the robokopkg url need to remove the trapi version?
src/service_aggregator.py
Outdated
| @@ -1182,7 +1258,7 @@ async def omnicorp(message, params, guid) -> (dict, int): | |||
| with open("to_omni.json","w") as outf: | |||
| json.dump(message, outf, indent=2) | |||
|
|
|||
| url = f'{os.environ.get("RANKER_URL", "https://aragorn-ranker.renci.org/1.4/")}omnicorp_overlay' | |||
| url = f'{os.environ.get("RANKER_URL", "https://aragorn-ranker.renci.org/1.5/")}omnicorp_overlay' | |||
There was a problem hiding this comment.
More trapi version urls
| @@ -174,7 +173,7 @@ async def entry(message, guid, coalesce_type, caller) -> (dict, int): | |||
| # We told the world what we can do! | |||
| # Workflow will be a list of the functions, and the parameters if there are any | |||
|
|
|||
| read_from_cache = bypass_cache or overwrite_cache | |||
| read_from_cache = not (bypass_cache or overwrite_cache) | |||
There was a problem hiding this comment.
So now the variable name makes sense, but the logic below is wrong. If read_from_cache is true, then we want to get cached results.
There was a problem hiding this comment.
Oof, I'm really swimming upstream on this one.
|
Since this isn't merged in yet, can we bump the reasoner-pydantic version to 5.0.2 please? |
|
OK, it's bumped - |
While the name of the PR says promiscuity, the actual code changes are all about going to trapi 1.5