-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add example functions
section to api-server
#7398
base: master
Are you sure you want to change the base?
Add example functions
section to api-server
#7398
Conversation
functions
section to api-server
functions
section to api-serverfunctions
section to api-server
@wvangeit please let me know if anything in this example is unclear. I am very happy to extend it to make it more realistic or add some actual functionality if required. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7398 +/- ##
==========================================
- Coverage 87.31% 85.14% -2.17%
==========================================
Files 1712 1348 -364
Lines 66429 56072 -10357
Branches 1125 578 -547
==========================================
- Hits 58004 47745 -10259
- Misses 8105 8142 +37
+ Partials 320 185 -135
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
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'm a bit lost. What would be te final goal of this functions?
I see no reference to that
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 am also not clear what this "function" api is. all this seems very hard-coded in the end... I thought this was something where you send a "function" to be executed in there... but this looks like you have to implement each function precisely instead...
services/api-server/src/simcore_service_api_server/api/routes/functions.py
Outdated
Show resolved
Hide resolved
...ages/service-library/src/servicelib/rabbitmq/rpc_interfaces/webserver/functions/functions.py
Outdated
Show resolved
Hide resolved
services/web/server/src/simcore_service_webserver/functions/_controller_rpc.py
Outdated
Show resolved
Hide resolved
services/web/server/src/simcore_service_webserver/functions/_controller_rpc.py
Outdated
Show resolved
Hide resolved
services/web/server/src/simcore_service_webserver/functions/_service.py
Outdated
Show resolved
Hide resolved
from . import _controller_rpc | ||
|
||
|
||
def setup_functions(app: web.Application): |
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.
setup docorator is missing here! See other setup to check this
@app_module_setup(
__name__,
ModuleCategory.ADDON,
settings_name="WEBSERVER_FUNCTIONS",
logger=_logger,
)
def setup_functions(app: web.Application) -> bool:
...
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.
Another thing that is missing is to create a settings field and mark is as DEV-FEATURE for this domain e.g.
class ApplicationSettings(BaseApplicationSettings):
# other fields
WEBSERVER_FUNCTIONS: Annotated[
bool,
Field(
json_schema_extra={"auto_default_from_env": True, _X_DEV_FEATURE_FLAG: True}
),
] = False
services/web/server/src/simcore_service_webserver/functions/_controller_rpc.py
Outdated
Show resolved
Hide resolved
services/api-server/src/simcore_service_api_server/api/routes/functions.py
Outdated
Show resolved
Hide resolved
@@ -44,6 +45,7 @@ def create_router(settings: ApplicationSettings): | |||
router.include_router( | |||
licensed_items.router, tags=["licensed-items"], prefix="/licensed-items" | |||
) | |||
router.include_router(functions.router, tags=["functions"], prefix="/functions") |
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 would then also enable this if DEV_FEATURE is enabled.
The idea is that as soon as @wvangeit merges, it can be used in master
Yes, sorry. That's on me. I did not explain well in the header what exactly the goal of this PR is. I will update it |
Yes, actually this PR just introduce some "dummy" functions so Werner can start implementing the actual api and has a pattern to follow. I have updated the header of this PR with some more info |
functions
section to api-serverfunctions
section to api-server
|
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.
ok thanks for the explanation. I am still a bit unsure. but let's see.
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.
Sorry, but I have doubts and issues with this approach. Please let's have chat about it. See my reasons below.
I am not convinced at all by these changes/pattern.
I believe that the current approach will just cause harm in the long run.
I see the following issues/uncertainties:
- I did not understand the concept of a
function
- no generic interface to register, launch and get the result of a function
- a function could also be long running (long lived), the current pattern will eventually fail (example, if the webserver is restarted)
- why was the webserver chosen as the owner of these functions?
- Can you please provide a schema/diagarm of how the interaction with a
functions
will work?
What's the point of this PR?
As part of out metamodeling efforts we plan to develop the concept of a
function
in the api server. These functions could point to concepts we already have in the api-server (studies or solvers) or to new types of functions. This PR does nothing more than setup a few files and connect afunctions
section in the api-server with a newfunctions
domain in the api-server so that @wvangeit can start implementing the thing. I.e. it is a skeleton to guide the development. I.e. the functions which are introduced in this PR will eventually go away as we start to merge the real functionality for thefunctions
.What do these changes do?
functions
section in api-server. Expose a singleping
endpoint as an exampleping
endpoint with webserver via rpc to webserver.Related issue/s
functions
api with a single endpoint which is connected all the way to the webserver via rpc #7348How to test
Dev-ops checklist