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

Move PyNetDicom to twisted or another async io framework #527

Open
chsasank opened this issue Aug 5, 2020 · 12 comments
Open

Move PyNetDicom to twisted or another async io framework #527

chsasank opened this issue Aug 5, 2020 · 12 comments

Comments

@chsasank
Copy link
Contributor

chsasank commented Aug 5, 2020

In #491, @scaramallion discussed removing python threads and using aysnc io/networking framework like twisted. We were looking to use pynetdicom in our project and we arrived at the same conclusion as @scaramallion. We have gone through twisted and it has great support to create protocols like SSH.

I know DICOM is lot more complicated than say, SMTP - All the more reason to not write low level code. We should leverage as much python ecosystem as possible.

Is your feature request related to a problem? Please describe.

Right now, pynetdicom depends a lot on python threads. It has code to handle race conditions, code to do transport at low level etc. Interface to use pynetdicom is lot difficult to use compared to other protocols because of this.

Describe the solution you'd like

There's a lot of code in pynetdicom whose functionality can be offloaded to twisted or some other framework. Doing this improves maintainability, readability and testability of the repo.

Describe alternatives you've considered

Twisted is great. It has built in support for lot of features required to write very clean code for new protocols. Twisted comes with its own event loop reactor, tcp/ip support allowing us to write proper servers and clients and lot more

These paradigms map fairly well to things like association, SCU, SCP etc.

Another advantage with twisted is we can write this interface called ASGI and then start using proper web frameworks like Django. Django channels has done just this using their channels and daphne projects for websocket protocol.

Additional context

We're willing to do most of the heavy lifting if maintainers can help us with the design that is acceptable to them.

cc: @yvd

@scaramallion
Copy link
Member

I've done very little research so far into what I'd like to use to replace threading so I don't have an opinion on your proposal one way or the other but twisted is definitely one of the candidates. I think it's great that you want to work on it though and I'm happy to provide advice if you want it.

@chsasank
Copy link
Contributor Author

Great!

I'm happy to provide advice if you want it.

Awesome. We'd certainly want some advice. Although we have a lot of background on web, our idea of dicom networking is limited. It'd certainly help if you can let us know where we can start from pynetdicom point of view.

@scaramallion
Copy link
Member

How would ASGI work with the DICOM TCP protocol? As I understand it, ASGI only applies to HTTP or websockets, not TCP.

@chsasank
Copy link
Contributor Author

ASGI is designed to be work for both synchronous and asynchronous protocols - unlike WSGI. Although most of the protocol is written for HTTP and websocket, we can extend it to any protocol. ASGI is just a simple reference (as opposed to a library) about how protocol functions should look like.

Having said that, ASGI is notch above what we have to do (in perspective of the OSI stack). We have to first tackle replacing python threading with twisted in pynetdicom. ASGI interface is fairly easy to build on top of this.

@scaramallion
Copy link
Member

Hmm, OK, thanks for the explanation. It does look like it'd be a nice extra though.

@chsasank
Copy link
Contributor Author

We have suggested twisted primarily because of its documentation for tcp/ip protocols. However, twisted has become somewhat 'oldschool'. I'm no expert in systems programming but trio seems quite interesting. What do you think is better - trio or twisted - for our use case?

@scaramallion
Copy link
Member

I have no real experience with either so no idea. I'm only just starting to experiment by attempting to write a cut-down association client and server using the various options.

@chsasank
Copy link
Contributor Author

When I get time, I'll check trio out and I'll let you know my findings. I highly recommend the first tutorial of twisted: https://twistedmatrix.com/documents/current/core/howto/servers.html

@moloney
Copy link
Contributor

moloney commented Apr 12, 2021

I have released a related package dcm that provides a high level asyncio API on-top of pynetdicom (basically entirely contained in the net module).

I run the pynetdicom routines in worker threads and use janus to provide a queue that provides both sync / async access. This obviously isn't ideal, but actually works reasonably well. Having an async API in pynetdicom itself would be great, but I do hope that compatibility with the asyncio eco-system is a priority. In this regard I think Twisted might be better than Trio (if an additional library is used), but I also see that Trio has an escape hatch in trio-asyncio so maybe it is a non-issue.

One lesson I have learned from writing this code is that in the world of Python async, context managers are critical to doing cleanup due to the issues outlined in PEP 533.

This was referenced Jun 5, 2021
@phistep
Copy link

phistep commented Oct 30, 2023

What is the status here? #642 seems to have gone stale, https://github.com/RadiotherapyAI/aiopynetdicom has not been updated in two years, 2023 Python asyncio has improved quite a bit since 2021. Any plans to continue this effort? Any way I could help out?

@scaramallion
Copy link
Member

I would say it's definitely stale. I support the effort if someone wants to try to implement it, but I don't think I can contribute much time to doing so.

@medihack
Copy link

medihack commented Nov 6, 2023

I am also quite interested in that. Currently, we use pynetdicom in a separate thread in an asyncio application and are pretty happy with it. Performance is good enough, and the reliability is excellent. I wonder a bit what to expect from a re-write using asyncio. More performance (any ideas on what scale?) or improved code maintenance (the asyncio streaming API is quite nice)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants