RTXteam / RTX

Software repo for Team Expander Agent (Oregon State U., Institute for Systems Biology, and Penn State U.)
https://arax.ncats.io/
MIT License
33 stars 21 forks source link

Does KG2 need meta_kg_info? #2148

Closed edeutsch closed 1 year ago

edeutsch commented 1 year ago

One thing that might be somewhat wasteful:

Both KG2 and ARAX run all the same BackgroundTasker things. Including fetching of meta_kgs from all KPs. Is this necessary for the KG2 service? OR can we disable meta_kg fetching for KG2?

amykglen commented 1 year ago

good point - KG2 doesn't have any need for meta KG or SmartAPI info. and in fact, we already skip loading both of those caches when in RTXKG2 mode (here).

so I think all we need to do is stop the background tasker from running those tasks on KG2 endpoints.

saramsey commented 1 year ago

Amy can you please remove that code from

https://github.com/RTXteam/RTX/blob/master/code/UI/OpenAPI/python-flask-server/KG2/openapi_server/__main__.py

saramsey commented 1 year ago

@amykglen please let me know if you have questions

edeutsch commented 1 year ago

I'm wondering what code you want to remove from main.py

amykglen commented 1 year ago

I'm in the same boat as @edeutsch - what code in that file do you mean, @saramsey? I don't think we want to entirely stop running the BackgroundTasker on KG2 endpoints, right? I was assuming maybe we need to pass a parameter of some sort to BackgroundTasker that tells it whether we're in KG2 mode or not, and if we are, it could skip refreshing the KP info caches.

saramsey commented 1 year ago

Ah, I didn't understand. Thank you for explaining.

So, we just want Background Tasker to not run the kp_info_cacher.py on KG2 endpoints, right? But we still want Background Tasker to keep running so it does its other jobs (e.g.., Query tracking?). Makes sense. I'm going to delete my previous suggestion to @amykglen in that case.

saramsey commented 1 year ago

@amykglen for this issue, I have created a branch issue-2148. I will attempt to code a fix in that branch.

saramsey commented 1 year ago

Fix committed. Testing now on /beta and /kg2beta.

saramsey commented 1 year ago

Testing looks good. Merging to master. YOLO.

saramsey commented 1 year ago

Rolling master out to /test and /kg2test.

saramsey commented 1 year ago

Issue looks fixed. @amykglen @edeutsch please let me know if you see any issues.

Summary: KP info cacher is no longer running on KG2 endpoints, but the background tasker is running (it just skips the KP info cacher stuff). Rolled out to /test, /kg2test, /beta, and /kg2beta. I recommend rolling it out to /devED and /devLM but I'll wait for your permission before doing that.

saramsey commented 1 year ago

Deleting the branch issue-2148 as it has been merged to master.