Timeout param #427
Timeout param #427
Conversation
strider/knowledge_provider.py
Outdated
| @@ -47,6 +48,8 @@ def __init__( | |||
| logger=logger, | |||
| preproc=self.get_preprocessor(kp["details"]["preferred_prefixes"]), | |||
| postproc=self.get_postprocessor(WBMT.entity_prefix_mapping), | |||
| max_batch_size=1, | |||
There was a problem hiding this comment.
Have we thoroughly tested how not batching will impact performance?
There was a problem hiding this comment.
Good catch. This snuck in cause I have other changes for batch sizing. I've removed it.
strider/server.py
Outdated
| lookup(query_dict, qid), timeout=max_process_time | ||
| # get max timeout | ||
| timeout = max( | ||
| max_process_time, query_dict.get("parameters", {}).get("timeout_seconds", 0) |
There was a problem hiding this comment.
I forget the complete details for fastAPI query param parsing. Is it possible for timeout_seconds to end up here as a string or some other non-numeric type.
There was a problem hiding this comment.
This isn't a query param, it's in a dedicated parameters field in the request body on the same level as message. And there's no type checking for timeout_seconds, so someone could pass whatever they wanted. parameters isn't part of the TRAPI spec, so I highly doubt anyone outside of ranking-agent devs are going to even know that this exists.
There was a problem hiding this comment.
Added type checks
Adds a check for
timeout_secondsinside of requestparameterssent from Aragorn. This will enable Strider to take longer for kp lookups to provide more thorough answers that get cached in Aragorn.We're also adding a
/clear_cacheendpoint so someone can easily clear the onehop cache.