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

Python server demo #272

Merged
merged 22 commits into from
Sep 14, 2023
Merged

Python server demo #272

merged 22 commits into from
Sep 14, 2023

Conversation

floryst
Copy link
Collaborator

@floryst floryst commented Feb 7, 2023

This adds a functional Python server to VolView through the "Remote Functions" tab.

Future demonstrations would include remote-loading of datasets and measurements.

  • add documentation on how to use and extend the server
  • support calling server APIs
  • support accessing client stores
  • UI for configuring connected server
  • demos for synchronous and asynchronous server RPC endpoints
  • server supports running standalone or as part of an ASGI framework
  • added application debug messaging
  • create button-click installer for python server (automated or leads to webpage for download/docs)

@floryst floryst requested review from aylward and PaulHax February 7, 2023 19:40
@netlify
Copy link

netlify bot commented Feb 7, 2023

Deploy Preview for dicomweb ready!

Name Link
🔨 Latest commit 459319f
🔍 Latest deploy log https://app.netlify.com/sites/dicomweb/deploys/63e2a92bf0c8070008eff118
😎 Deploy Preview https://deploy-preview-272--dicomweb.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Feb 7, 2023

Deploy Preview for volview ready!

Name Link
🔨 Latest commit 4f5eb20
🔍 Latest deploy log https://app.netlify.com/sites/volview/deploys/63ea747aad4d430008552a30
😎 Deploy Preview https://deploy-preview-272--volview.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

server/custom/user_api.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@PaulHax PaulHax left a comment

Choose a reason for hiding this comment

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

Amazing, succinct, magic!

server/README.md Outdated Show resolved Hide resolved
server/README.md Outdated Show resolved Hide resolved
server/README.md Outdated Show resolved Hide resolved
server/README.md Outdated Show resolved Hide resolved
server/README.md Outdated Show resolved Hide resolved
server/volview_server/api.py Outdated Show resolved Hide resolved
src/components/ModulePanel.vue Outdated Show resolved Hide resolved
src/components/ServerModule.vue Outdated Show resolved Hide resolved
src/components/ServerModule.vue Show resolved Hide resolved
src/core/remote/codecs.ts Outdated Show resolved Hide resolved
@floryst
Copy link
Collaborator Author

floryst commented Feb 13, 2023

@aylward I've updated the demo with a streaming progress example.

server/custom/user_api.py Outdated Show resolved Hide resolved
@netlify
Copy link

netlify bot commented May 24, 2023

Deploy Preview for volview-dev ready!

Name Link
🔨 Latest commit 0296c0b
🔍 Latest deploy log https://app.netlify.com/sites/volview-dev/deploys/6501db4a21d9bc0008b48b48
😎 Deploy Preview https://deploy-preview-272--volview-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@floryst
Copy link
Collaborator Author

floryst commented Jun 14, 2023

The python server has been updated and improved. Check out the user_api.py file for how to use the new API. Details of the new changes:

  • switched from wslink to socket.io for RPC. After chatting with @jourdain, we decided against extending wslink outside its current feature scope.
  • added the ability to access client stores from the server. This includes accessing state and calling actions. The median filter example demonstrates this usage.
  • Renamed the rpc decorator to expose. Additionally, the stream decorator has been merged into expose.
  • Both the client and server now support custom serializers and deserializers. Previously, only the server supported them.

General VolView changes:

  • Dropped the $id store dependency, as it conflicted with pinia's $id property. Instead, an ID store is now available.

server/pyproject.toml Outdated Show resolved Hide resolved
server/pyproject.toml Show resolved Hide resolved
@floryst
Copy link
Collaborator Author

floryst commented Jun 16, 2023

I've lowered the python requirement to 3.8.1 and updated the dependencies list.

@phtran-dev
Copy link

Hi @floryst ,
Would you mind sharing your reason of switching from wslink to socket.io ? Does wslink not suffice the requirements of Volview Server ? In addition, I saw many Framework like : FastAPI or Sanic have Socket feature. Is this possible to integrate them into Volview ?
Thanks

@floryst
Copy link
Collaborator Author

floryst commented Jun 23, 2023

Hi @phtran-dev,

One limitation of wslink is that bi-directional RPC is not currently supported, which is necessary to support accessing the client state store from the server. While I could technically add a bi-directional RPC mechanism on top of wslink, it's another layer of complexity and lacks implementation symmetry when compared to using socket.io.

Another consideration is that python-socketio is ASGI compatible, so you can integrate this python server into your ASGI-compatible framework (e.g. sanic or fastapi) or server (e.g. uvicorn). I still need to change the docs to using the volview server as an ASGI middleware. I suppose wslink can be made ASGI compatible (if it isn't already), so this is less of a concern.

@phtran-dev
Copy link

phtran-dev commented Jun 23, 2023

Hi @floryst ,
Thanks a lot for your quick response. That would be great ideas from your team while making Volview be compatible with frameworks supporting ASGI (like FastApi or Sanic). I have another question. My team is implementing a 3D Server which employs wslink as transportation and vtk and backend service. The client side is also using vtk.js which supports wslink. The result of project is quite good. However, if I switch to use your Volview framework (where I can reuse your socket transportation), does it need to change the client code ? I am asking because I do not think the vtk.js does support the socket.io . Thanks in advance.

@jourdain
Copy link

Another point that is important to keep in mind is the fact that wslink is really meant to be used with a dedicated server process per client. And for his reference implementation, @floryst wanted to have a simple server (1 instance that serves many clients). But in general, it might be easier to abstract the VolView client side so a wslink version of that client (with a dedicated server process) could be swapped and used.

@floryst do you think you can make sure the client side could be swapped? That way @phtran-dev can still rely on his existing infrastructure by just adding a protocol on his server side.

@floryst
Copy link
Collaborator Author

floryst commented Jun 26, 2023

I'm not sure about a swappable client yet. My suggestion would be to use an ASGI-compatible framework and register the wslink and volview servers under different routes (e.g. /wslink routes to the wslink server and /volview routes to the volview server). You'll also probably need to do path stripping before passing requests onto wslink or volview.

@phtran-dev
Copy link

phtran-dev commented Jun 27, 2023

Hi @floryst ,
I am interested in your architecture where one service can serve multiple socket connections (which is contradict to wslink where one server serves only one socket). However the existing VTK.js (client side) is heavily relying on the wslink to communicate with the server as in this example [https://kitware.github.io/vtk-js/examples/RemoteView.html#Source] . So do we need to modify the source code of VTK.js to be compatible with your new Volview server ? Do you have any example of rendering 3D image using your Volview ?

@aylward
Copy link
Contributor

aylward commented Jun 30, 2023

The VUE_APP_REMOTE_SERVER_URL variable in the .env file should also be available in the setting dialog. If this variable must be defined when the app is launched, then changing the variable in the dialog should cause another pop-up that gives the user a button to reload/restart volview with the new setting applied.

If a .env file doesn't exist, it should be automatically created - when running from github source.

Instead of displaying tabs (e.g., DICOM WEB and Remote Functions) based on a variable being set, we should test the variable and if they work, display the tabs, otherwise don't. Could even put "test" buttons next to those variables in the settings dialog.

@floryst
Copy link
Collaborator Author

floryst commented Jul 5, 2023

@aylward I force-pushed this branch and updated it to match main.

  • VUE_APP_REMOTE_SERVER_URL is now VITE_REMOTE_SERVER_URL, so you will need to update your .env accordingly
  • Documentation has been moved to the documentation/ directory. @phtran-dev this includes some deployment strategies that integrate with existing servers.

@phtran-dev VolView currently doesn't support remote rendering. The server is just for remote data processing and control, so it won't conflict at all with your current remote rendering setup.

@PaulHax
Copy link
Collaborator

PaulHax commented Jul 11, 2023

I can only get the median filter to work on the "MRI HEART ... Ax FIESTA non gated" sample data. Running the filter on the other sample images hangs the Python server on the img = await store.dataIndex[base_image_id] line.

https://github.com/Kitware/VolView/pull/272/files#diff-c2c4c80c7d8530042fff10f2bee85bb288a3bd85a2f160ef7f54e8ad07ab5d01R70

Chrome on Ubuntu

@jadh4v
Copy link
Collaborator

jadh4v commented Jul 12, 2023

I can only get the median filter to work on the "MRI HEART ... Ax FIESTA non gated" sample data. Running the filter on the other sample images hangs the Python server on the img = await store.dataIndex[base_image_id] line.

https://github.com/Kitware/VolView/pull/272/files#diff-c2c4c80c7d8530042fff10f2bee85bb288a3bd85a2f160ef7f54e8ad07ab5d01R70

Chrome on Ubuntu

Same for me on Windows 11 with Edge browser. Median filter works on the image Paul mentioned above but not for any other images.

This adds support for bidirectional python server integration with
VolView.
- Adds medianFilter example
- Remove unused msgpack dependency
- add charset-normalizer dependency
- client is accessible via the server store
- adds support for restricting client-side property access
- add debug logging
- remove max message size option, since we now chunk
- fix handling of image direction
- Blob/File.size, not .length
- ensure cause is an error
- remove non-relevant doc
- Simplifies building VolView APIs, while retaining support for
  class-based APIs.
- Re-worked examples to show both the simple and the class-based APIs.
- get_current_session() associates session objects with clients.
- median filter example is no longer blocking.
- FastAPI example in docs and in examples/ directory
- Add support for custom socket.io paths
@floryst
Copy link
Collaborator Author

floryst commented Sep 13, 2023

I aim to merge this in without exposing the Remote tab for now. This will get the core components in, and we can add the Remote tab later once we have everything else in place for it.

@jadh4v
Copy link
Collaborator

jadh4v commented Sep 13, 2023

I aim to merge this in without exposing the Remote tab for now. This will get the core components in, and we can add the Remote tab later once we have everything else in place for it.

Please do check-in the updated remote tab component though, so we can understand the usage for downstream LungAir.

@floryst
Copy link
Collaborator Author

floryst commented Sep 13, 2023

Please do check-in the updated remote tab component though, so we can understand the usage for downstream LungAir.

Yep, the ServerModule.vue file is being committed, just not used.

@floryst floryst merged commit bed260c into main Sep 14, 2023
7 checks passed
@floryst floryst deleted the python-server branch September 14, 2023 15:38
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.

6 participants