-
Notifications
You must be signed in to change notification settings - Fork 16
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
Service and sandbox orchestrator #7
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like I need a refresher on how exactly all this code fits together but overall LGTM.
|
||
return wrapper | ||
|
||
if func is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the use case where func
is None. When would this come up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just means when the decorator is called on its own it doesn't throw an error. I don't know when it would come up either 🤷♀️
log.warning( | ||
f"{list(kwargs.keys())} is not a valid argument and will not be used; use 'service_config'." | ||
) | ||
service_config = arg if arg is not None else kwargs.get("service_config", {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need any additional check regarding the form of arg
if we are going to use it as service_config?
service_route_count = 0 | ||
client_count = 0 | ||
|
||
for name in dir(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit heavily nested. Ideally, I'd prefer to use a list comprehension to work out what should go in the for loop and avoid the nested if.
called_attrs= [getattr(self, name) for name in dir(self) if callable(getattr(self, name)]
But this isn't essential.
What is the purpose of this PR?
This PR adds the sandbox and service orchestration layer through the use of decorators
@api
and@sandbox
.Users can decorate their function with the
@api
decorator to mark it as the primary A/LLM processing function of their service. The decorator will mark the function as a route to a FastAPI app and mount it to the correct endpoint as defined in theUseCase
.The
@sandbox
decorator marks the entire class as a sandbox orchestrator. It will set up the client and service with the configurations defined inService
,Client
, andUseCase
. The class must be derived fromBaseUseCase
, i.e. inherit fromClinicalDecisionSupport
. Typically, The@ehr
function will define the synthetic data the users want to test with, and@api
function will be where the actual processing logic lies..start_sabdbox()
will then start the service and client on separate async threads.Example use:
Next steps
Integrating with actual data generator (it needs to also include meta data for the
CDSRequest
)Export request/responses to a directory, or visualise through a streamlit dashboard