django / daphne

Django Channels HTTP/WebSocket server
BSD 3-Clause "New" or "Revised" License
2.32k stars 256 forks source link

Make DaphneProcess pickleable #440

Closed adamchainz closed 1 year ago

adamchainz commented 1 year ago

Part of a fix for https://github.com/django/channels/issues/1921.

Make DaphneProcess pickleable by:

  1. Changing application argument to get_application, a callable that returns an application. If this is pickleable, the process will be.
  2. Removing clever use of lambda for plain-old None pattern.
adamchainz commented 1 year ago

I didn't realize DaphneProcess is used elsewhere in Daphne, in BaseDaphneTestingInstance. If you accept this approach, I can fix up that class, which I think is mostly used in Daphne's own tests.

carltongibson commented 1 year ago

This would let us remove the start method workaround right? In which case yes let's see if we can get it right.

(That's the correct fix: I was merely applying the bandaid before.)

Thanks Adam!

adamchainz commented 1 year ago

Yes it would.

carltongibson commented 1 year ago

Let's have it then. 😀

If you want me to think harder before you take it on, leave it a couple of days for me to have a look at. Otherwise press on. (As you prefer)

carltongibson commented 1 year ago

Hey @adamchainz — I just adjusted to make BaseDaphneTestingInstance.__enter__ do the right thing. Can you take a look? Thanks!

adamchainz commented 1 year ago

Yeah seems good.

carltongibson commented 1 year ago

Thanks for looking so quickly @adamchainz 🥳