search

NCATSTranslator / Relay

Autonomous relay system for NCATS Biomedical Data Translator
MIT License
5 stars 24 forks source link

ARS queries a subset of KPs? #501

Open edeutsch opened 1 year ago

edeutsch commented 1 year ago

Something I've often wondered: the ARS seems to query some KPs, but only a subset. Is there some intention to the subset of KPs that are queried? Do we want ARS to query them all? Do we want ARS to query none? Or is the current subset still good?

Many seem to always return errors? It seems to me that MolePro is usually the only one that usually returns a non-error.

e.g.: https://arax.ci.transltr.io/?r=0bf293be-7e4c-4057-ab50-7f34670e5b61 image

gloriachin commented 1 year ago

Multiomics KPs has been digested through ara-bte, do we need to pull them separately? If so, there are the some of the endpoints:

Multiomics BigGIM-DrugResponse

"https://bte.test.transltr.io/v1/smartapi/adf20dd6ff23dfe18e8e012bde686e31/query"

Multiomics wellness

"https://api.bte.ncats.io/v1/smartapi/02af7d098ab304e80d6f4806c3527027/query"

Multiomics clinical trial

"https://api.bte.ncats.io/v1/smartapi/d86a24f6027ffe778f84ba10a7a1861a/query"

ShervinAbd92 commented 1 year ago

Hi @edeutsch which subset of kps are we not hitting? the ones that return 598 error are mainly due to getting timedout after 5 min.

edeutsch commented 1 year ago

Spoke is one, see https://github.com/NCATSTranslator/Feedback/issues/523

In total there are 32 TRAPI KPs listed here: https://arax.ci.transltr.io/?smartapi=1 but ARS appears to be querying 8. I'm wondering about the discrepancy between 8 and 32.

If the 598 means that these services are timing out after 5 minutes, that would imply that 6 out of 8 KPs the ARS queries are routinely (always?) timing out after 5 minutes. That would seem to be bad, but is never discussed.

ShervinAbd92 commented 1 year ago

looking at the ARS structure i see that we only have 8 of them registered but we should be able to add them, unless there is a reason we haven't done so far. @MarkDWilliams what do you think about that?

edeutsch commented 1 year ago

On the one hand, I'm guessing the KP responses are not used for results returned to the UI, so there's no need for the ARS to be querying KPs at all On the other hand, it's nice to be able to see how the KPs are responding to the queries from a testing perspective On the other err... foot, we're routinely testing a subset of KPs and most of them always appear to fail but there's no action on that. On the other foot, if we want to test KPs, seems like it would be useful to test them all.

MarkDWilliams commented 1 year ago

I think in production at least, I would lean towards deregistering all the KPs as they aren't really necessary for anything beyond debugging. In the lower environments, I am more split. There is some value to having them for testing, but really only for testing. So, maybe they should only be called if specified? Not sure the best way to implement that switch just yet.

maximusunc commented 1 year ago

My two cents are that the ARS shouldn't be the one "testing" kps, but that it should be a separate more dedicated test.

edeutsch commented 1 year ago

I would also vote that the normal mode of ARS operation would only be to query active ARAs.

If there is a separate special mode to query all agents including all KPs, that would be interesting and useful, but I suggest not routine operation.