Functions draft, dontmerge ✨ 🗃️ #7539
Conversation
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7539 +/- ##
==========================================
- Coverage 87.48% 87.16% -0.33%
==========================================
Files 1742 1678 -64
Lines 67432 65991 -1441
Branches 1144 1048 -96
==========================================
- Hits 58990 57518 -1472
- Misses 8121 8174 +53
+ Partials 321 299 -22
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Thanks a lot for the great effort Werner! It looks very cool.
I suggest several minor changes, but I have one big request and that concerns running the function. I think it is crucial that that happens asyncronously and in one of our dedicated celery workers. This call (in the case of studies) was exactly what was blocking us last time we did the metamodeling. We are in a better position now because we have a job scheduling framework for handling "internal long running tasks". We should use it! 😁
packages/postgres-database/src/simcore_postgres_database/models/functions_models_db.py
Show resolved
Hide resolved
packages/postgres-database/src/simcore_postgres_database/models/functions_models_db.py
Show resolved
Hide resolved
packages/postgres-database/src/simcore_postgres_database/models/functions_models_db.py
Show resolved
Hide resolved
...ages/service-library/src/servicelib/rabbitmq/rpc_interfaces/webserver/functions/functions.py
Show resolved
Hide resolved
...ibrary/src/servicelib/rabbitmq/rpc_interfaces/webserver/functions/functions_rpc_interface.py
Show resolved
Hide resolved
| ) | ||
| else: # noqa: RET505 | ||
| msg = f"Unsupported function class: [{function.function_class}]" | ||
| raise TypeError(msg) |
| ) | ||
| else: # noqa: RET505 | ||
| msg = f"Unsupported function class: [{returned_function_job.function_class}]" | ||
| raise TypeError(msg) |
services/web/server/src/simcore_service_webserver/functions/_functions_controller_rpc.py
Show resolved
Hide resolved
services/web/server/src/simcore_service_webserver/functions/_functions_controller_rpc.py
Show resolved
Hide resolved
| ) | ||
| else: # noqa: RET505 | ||
| msg = f"Unsupported function class: [{returned_function_job.function_class}]" | ||
| raise TypeError(msg) |
There was a problem hiding this comment.
this error should probably be propagated to the api-server
Yeah, sure makes sense. I'm basically calling the study_job api. It's probably best to implement the celery changes there? (it they're not there yet) |
| description: str | None = None | ||
| input_schema: FunctionInputSchema | None = None | ||
| output_schema: FunctionOutputSchema | None = None | ||
|
|
There was a problem hiding this comment.
Wrt the nullable uuid I suggest the following change:
class Function(BaseModel):
title: str
description: str
input_schema: FunctionInputSchema
output_schema: FunctionOutputSchema
class RegisteredFunction(Function):
uuid: UUID| description: str | None = None | ||
| input_schema: FunctionInputSchema | None = None | ||
| output_schema: FunctionOutputSchema | None = None | ||
|
|
There was a problem hiding this comment.
Typically it pays off in the long run to go for non-nullable fields, so unless there really is a good reason for them not to be there it is best to require them. At least that's my opinion
packages/models-library/src/models_library/api_schemas_webserver/functions_wb_schema.py
Show resolved
Hide resolved
| function_uid: FunctionID | ||
| inputs: FunctionInputs | None | ||
| outputs: FunctionOutputs | None | ||
|
|
There was a problem hiding this comment.
I think you just need to inherit from the ApiServerInputSchema and it will give you the correct formatting of the schema. I don't see why that doesn't give you the flexibility you want?
|
|
||
| if row is None: | ||
| msg = "No row was returned from the database after creating function." | ||
| raise ValueError(msg) |
There was a problem hiding this comment.
Yes, an assert could also make sense here. In that case, remember to append # nosec so we can disable those in production some day in case we want to.
This a RFC for the functions API. Mostly to get feedback from Mads/Pedro.
I'm aware that there are still quite a few things to be improved/fixed in this branch.
Examples of the most obvious:
What do these changes do?
This introduces the Functions API.
Functions are objects that can be in the backend: projects, solvers, python code, etc ...
Users can create their own functions from the relevant objects.
They accept parameters and return results.
Each function run creates a function job. These function job can also act as a cache to prevent rerunning a function with the same parameters.
Related issue/s
#7506
How to test
TBD
A high-level functional test script is here:
https://github.com/wvangeit/osparc-functions-api-test/blob/master/test.py
Dev-ops checklist