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

ARS queries a subset of KPs? #501

Open
edeutsch opened this issue Sep 8, 2023 · 8 comments
Open

ARS queries a subset of KPs? #501

edeutsch opened this issue Sep 8, 2023 · 8 comments

Comments

@edeutsch
Copy link
Contributor

edeutsch commented Sep 8, 2023

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

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

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
Copy link
Contributor Author

edeutsch commented Sep 8, 2023

Spoke is one, see NCATSTranslator/Feedback#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
Copy link
Collaborator

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
Copy link
Contributor Author

edeutsch commented Sep 8, 2023

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

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

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants