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

Static REST endpoint mode for invoking CLI tasks #217

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

zachmullen
Copy link
Member

@zachmullen zachmullen commented Jun 30, 2023

This provides an off-by-default mode for Slicer CLI web with an alternate mode of supporting CLI task execution in share-nothing deployments. By setting GIRDER_STATIC_REST_ONLY=true in the environment, this mode is enabled. As written, it has the following limitations:

  • Tasks no longer appear as individual routes in the Swagger UI, and in fact it's no longer possible to execute them via the Swagger UI page, though that could possibly be changed by providing a catch-all form input for additional parameters. The existing item ID-based routes should work identically as before, though, which is what the existing front-end used.
  • The HTTP API routes that were based on the image name and CLI name no longer exist in this mode.
  • Batch execution is not supported in this mode, but that was mostly to limit the scope of this PR; it should theoretically be fine to add in this new mode, but I'd prefer to separate that work out.
  • It does not yet support the datalist operation.

With this change, the web server will now be able to execute newly-added Slicer CLI task items without the need for a manual server restart.

@zachmullen zachmullen force-pushed the static-rest-endpoint branch 2 times, most recently from 5cfb997 to 9d90439 Compare June 30, 2023 17:30
@@ -62,7 +65,35 @@ def __init__(self, name):

self.route('GET', ('path_match', ), self.getMatchingResource)

self._generateAllItemEndPoints()
if os.environ.get('GIRDER_STATIC_REST_ONLY'):
self.route('POST', (':id', 'run'), self.runCli)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The endpoints that are generated elsewhere are self.route('POST', ('cli', ':id', 'run'), self.runCli). I think the tests are passing because when we upload or refresh a CLI, it is still invoking addRestEndpoints which adds these.

Further, some CLIs can expose an point like POST cli/:id/datalist/:key which also need to be handled (this endpoint is used, for instance, by the MONAI label CLI docker to fetch names of available segmentation models).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks!

It's true that this is not yet tested.

@zachmullen zachmullen marked this pull request as draft July 3, 2023 15:18
@zachmullen
Copy link
Member Author

This branch is deployed here https://histomics-demo.com/api/v1#/slicer95cli95web

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

Successfully merging this pull request may close these issues.

None yet

2 participants