Django Integration

[Chad Paulson]

I was happy to read that part of the Mozilla Open Source Support program funding that was recently awarded to the Django Software Foundation will go toward integrating key components of Django REST Framework into Django.

Since I haven't found any update since the initial announcement in December, I was curious what the status of this integration is and, if possible, how soon it might be available.

https://www.djangoproject.com/weblog/2015/dec/11/django-awarded-moss-grant/

[Daniel Chimeno]

Hello,

There are more information about the project in the doc page:

https://channels.readthedocs.org/en/latest/

[Andrew Godwin]

Hi Chad,

The REST framework request feature integration, in particular, has not started yet; Mozilla has been having some issues working out how to pay their various grantees, and from my understanding we still haven't received the grant money yet.

Channels is well underway, but that's only because I'm able to do the work now and wait to get paid for it later; I don't expect the same of other contributors.

Andrew

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/2bdf074c-5e21-48cf-a93c-ba19f7744a89%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Tim Graham]

Hi Andrew,

What's your thinking about whether or not any of this will make it into 1.10? The alpha is scheduled for May 16.

[Andrew Godwin]

My intention is still to get it in - the external library is pretty much at the stable point now, and I'm preparing to brand it 1.0. Once I do that, I'll start work on the Django patch.

It's made a bit easier by the fact that the code is four repositories, three of which (asgiref, daphne, and asgi_redis) can continue to be separate modules. Only the code in the "channels" repository needs to go into Django itself, and while it's the biggest repo, most of it should slot right in; there's not really much in core itself that needs changing, more about addition and tweaking some imports.

Andrew

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/dbc29625-3af4-4b73-9b60-78d269352d52%40googlegroups.com.

[Asif Saifuddin]

Hi,

about the DRF integration in django I went through the dep and the branch of tom christies work. He shared his opinion as some one should do the work and he will provide guidance. I still have some interest in the REST parts work. If Tom christie/DSF core team allows.

Thanks

Asif

[Tim Graham]

Hi Andrew,

How are things going with the patch [0]? Do you think you might have it ready by next Monday or so, so that I'll have at least a few days to review it before the alpha scheduled for May 16?

[0] https://github.com/django/django/pull/6419

[Andrew Godwin]

I'm basically slowly fixing the test system failures (mostly because I introduced some new packages); the patch is ready to go code-wise, just need to get it green. Let me fix the current set now and see if that pushes it over the edge.

(There are some refinements the patch needs in terms of things like making the enforce_ordering decorator a bit more efficient, but I see these as being entirely valid things to do after the feature freeze)

Andrew

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/a5d0f33d-882f-4e02-9191-281594ff5522%40googlegroups.com.

[Mark Lavin]

I had assumed this was still a work in progress because there are missing tests and some documentation. The build is passing but the unittest coverage for the new modules seems low or at least not up to the standards I expect for Django contributions. The same for the daphne and asgiref packages which are new requirements. In previous discussions there were talks about performance benchmarks to track potential regressions before this would be accepted similar to the template-based widget rendering change. I don't see any references here or in this work. What is the status of those?

Best,

Mark

[Andrew Godwin]

On Wed, May 4, 2016 at 6:15 AM, Mark Lavin <markd...@gmail.com> wrote:

I had assumed this was still a work in progress because there are missing tests and some documentation. The build is passing but the unittest coverage for the new modules seems low or at least not up to the standards I expect for Django contributions. The same for the daphne and asgiref packages which are new requirements. In previous discussions there were talks about performance benchmarks to track potential regressions before this would be accepted similar to the template-based widget rendering change. I don't see any references here or in this work. What is the status of those?

- The documentation on Channels is considerably more than I would have accepted for a patch. What do you think is missing?

- The channels branch is likely missing some tests around auth and session in particular, but a lot of the "main" stuff has tests; what would you need before accepting it?

- asgiref quite literally has more tests than actual code we're using, so I don't see a problem there

- Daphne is severely lacking in tests for WebSocket encoding/decoding support as writing a test harness for it is ridiculously hard and I was hoping to get some help on that now we finally have the MOSS money around.

- There are no performance regression tests as by default the codepath in Django is the same as before (apart from runserver, which in my basic tests actually sped up); as long as you deploy using WSGI, you never even touch Channels code (which wasn't the original plan, but now is for sanity reasons).

I would like some leeway on the external dependencies being fully tested considering that they are not beholden to Django's release cycle, but maybe because we're declaring them as core dependencies (I patched in install_requires into setup.py for them) we should be very strict? If this patch is going to be denied based on Daphne not having sufficient test coverage then I suspect we'll miss the deadline and have to foist this on the LTS instead, which I really don't want.

Andrew

[Mark Lavin]

As noted in the PR there is at least one new setting, CHANNEL_SESSION_ENGINE, which is lacking documentation. The notes on deployment for "Running ASGI under WSGI" don't give any motivation why you would want to do this given that it doesn't support websockets in this case. Is there any advantages? Disadvantages?

There are tests for the routing and request handling but there are no unittests that I can see for the auth/session handling, Channel/Message/Group/ChannelLayerManager/custom JSON encoder classes and their usage, or the worker/runworker code. The changes to runserver aren't tested. The new test utilities aren't tested either (yes that's a thing). All new code should have tests. Isn't that Django's current standard?

asgiref has one "basic" test around WSGI handling which the usage is documented in this change. I don't think anyone would dare say that covers potential edge cases in WSGI/HTTP handling. Not test related, the status code map is also missing many HTTP status codes.

I'm not in favor of on leeway for external dependencies for the reason you note: they are in the install_requires. Django has avoided having required dependencies and I don't think it sets a good precedent to have the first set be those which don't meet the quality standards expected by the community. That is they don't have sufficient docs/tests to be include in Django itself. It isn't clear why these are required if you are claiming that they aren't required to run. If these we're in the install_requires then I would withdraw the objections about them with regard to this change in Django.

-Mark

[Andrew Godwin]

On Wed, May 4, 2016 at 8:26 AM, Mark Lavin <markd...@gmail.com> wrote:

As noted in the PR there is at least one new setting, CHANNEL_SESSION_ENGINE, which is lacking documentation.

That's been added now.

 

The notes on deployment for "Running ASGI under WSGI" don't give any motivation why you would want to do this given that it doesn't support websockets in this case. Is there any advantages? Disadvantages?

It lets you still have background tasks and non-WebSocket interface servers (or run websocket interfaces separately). I've expanded the docs to say this.

 

There are tests for the routing and request handling but there are no unittests that I can see for the auth/session handling,

Yes, these are missing.

 

Channel/Message/Group/ChannelLayerManager/custom JSON encoder classes and their usage,

The ASGI conformance test suite takes care of this: https://github.com/andrewgodwin/django/blob/channels/tests/channels_tests/test_database_layer.py#L3 (that's also used for the inmemory module and asgi_redis in their test suites)

 

or the worker/runworker code.

What would you suggest these tests look like? They can't spin up a worker and fire requests at it, and most of the worker code is interactive.

 

The changes to runserver aren't tested.

Runserver in Django only has a single test, presumably for the same reason; I don't know how I can write a test for this considering the autoreload and extra process mechanisms we'd want to cover.

 

The new test utilities aren't tested either (yes that's a thing).

I would say that the usage of those utilities in other tests means they are tested sufficiently, given that they're used sufficiently; writing a test for just the channel test case would look very similar to the basic tests elsewhere.

 

All new code should have tests. Isn't that Django's current standard?

There's no clear standard, the docs just say the tests should "exercise all of the new features". I admit that auth/session needs tests - I'll get those done this week - but I don't see how we can reasonably test the interactive commands in an automated fashion given the current test harness (and lack of tests for things like runserver) in Django.

 

asgiref has one "basic" test around WSGI handling which the usage is documented in this change. I don't think anyone would dare say that covers potential edge cases in WSGI/HTTP handling. Not test related, the status code map is also missing many HTTP status codes.

The WSGI handling part of asgiref is not considered complete, and I will probably remove the section about using it in the main Django docs. Only the inmemory backend should be considered used by Django.

 

I'm not in favor of on leeway for external dependencies for the reason you note: they are in the install_requires. Django has avoided having required dependencies and I don't think it sets a good precedent to have the first set be those which don't meet the quality standards expected by the community. That is they don't have sufficient docs/tests to be include in Django itself. It isn't clear why these are required if you are claiming that they aren't required to run. If these we're in the install_requires then I would withdraw the objections about them with regard to this change in Django.

With a change I'm going to make, Django will depend on these in install_requires but not actually use them until you try and use a channels feature; they're in install_requires out of user convenience (having a traceback the first time you use the new built-in Django feature seems bad). Considering we're launching Channels in 1.10 as a "provisional" feature, my opinion would be that we can go slightly easier on these dependencies (but of course I'm biased)

Andrew

 

On Wednesday, May 4, 2016 at 10:26:22 AM UTC-4, Andrew Godwin wrote:

On Wed, May 4, 2016 at 6:15 AM, Mark Lavin <markd...@gmail.com> wrote:

I had assumed this was still a work in progress because there are missing tests and some documentation. The build is passing but the unittest coverage for the new modules seems low or at least not up to the standards I expect for Django contributions. The same for the daphne and asgiref packages which are new requirements. In previous discussions there were talks about performance benchmarks to track potential regressions before this would be accepted similar to the template-based widget rendering change. I don't see any references here or in this work. What is the status of those?

- The documentation on Channels is considerably more than I would have accepted for a patch. What do you think is missing?

- The channels branch is likely missing some tests around auth and session in particular, but a lot of the "main" stuff has tests; what would you need before accepting it?

- asgiref quite literally has more tests than actual code we're using, so I don't see a problem there

- Daphne is severely lacking in tests for WebSocket encoding/decoding support as writing a test harness for it is ridiculously hard and I was hoping to get some help on that now we finally have the MOSS money around.

- There are no performance regression tests as by default the codepath in Django is the same as before (apart from runserver, which in my basic tests actually sped up); as long as you deploy using WSGI, you never even touch Channels code (which wasn't the original plan, but now is for sanity reasons).

I would like some leeway on the external dependencies being fully tested considering that they are not beholden to Django's release cycle, but maybe because we're declaring them as core dependencies (I patched in install_requires into setup.py for them) we should be very strict? If this patch is going to be denied based on Daphne not having sufficient test coverage then I suspect we'll miss the deadline and have to foist this on the LTS instead, which I really don't want.

Andrew

--

You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/0b519047-96ee-4ac9-9230-0f487625a7e2%40googlegroups.com.

[Markus Holtermann]

What about having asgiref and daphne as optional dependencies instead of
hard once and raising a proper exception "please install ..." when the
import fails?

```
try:
from asgiref import ...
except ImportError:
raise ImportError(
"Please ensure you installed asgiref to use this feature. Do so "
"with `pip install Django[channels]"
)
```

i.e. `pip install "Django[channels]"` ?

/Markus

>> <https://groups.google.com/d/msgid/django-developers/0b519047-96ee-4ac9-9230-0f487625a7e2%40googlegroups.com?utm_medium=email&utm_source=footer>
>> .

>>
>> For more options, visit https://groups.google.com/d/optout.
>>
>

>--
>You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
>To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
>To post to this group, send email to django-d...@googlegroups.com.
>Visit this group at https://groups.google.com/group/django-developers.

>To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAFwN1uqV3XNhpP45U0wVdGbHv65z1cSECemKAyB8O5dYV0%2B6pA%40mail.gmail.com.

[Andrew Godwin]

On Wed, May 4, 2016 at 10:06 AM, Markus Holtermann <in...@markusholtermann.eu> wrote:

What about having asgiref and daphne as optional dependencies instead of
hard once and raising a proper exception "please install ..." when the
import fails?

```
try:
   from asgiref import ...
except ImportError:
   raise ImportError(
       "Please ensure you installed asgiref to use this feature. Do so "
        "with `pip install Django[channels]"
   )
```

i.e. `pip install "Django[channels]"` ?

That is the other option, but it seems to run contrary to Django's traditional batteries-included philosophy (and would result in some ugly, ugly import code in the channels modules). I'm curious to hear more opinions though, if there's support for it I'm happy to change it.

Andrew 

[Mark Lavin]

On Wednesday, May 4, 2016 at 12:39:02 PM UTC-4, Andrew Godwin wrote:

On Wed, May 4, 2016 at 8:26 AM, Mark Lavin <markd...@gmail.com> wrote:

As noted in the PR there is at least one new setting, CHANNEL_SESSION_ENGINE, which is lacking documentation.

That's been added now.

 

The notes on deployment for "Running ASGI under WSGI" don't give any motivation why you would want to do this given that it doesn't support websockets in this case. Is there any advantages? Disadvantages?

It lets you still have background tasks and non-WebSocket interface servers (or run websocket interfaces separately). I've expanded the docs to say this.

 

Given that there is no guarantee of message delivery, I don't think this is suitable or recommended for using for background tasks. I don't think the Django docs should encourage that usage. At least without a strong warning that this isn't a good idea.

 

There are tests for the routing and request handling but there are no unittests that I can see for the auth/session handling,

Yes, these are missing.

 

Channel/Message/Group/ChannelLayerManager/custom JSON encoder classes and their usage,

The ASGI conformance test suite takes care of this: https://github.com/andrewgodwin/django/blob/channels/tests/channels_tests/test_database_layer.py#L3 (that's also used for the inmemory module and asgi_redis in their test suites)

 

Yes the code is covered by integration/functional tests but again I feel like there should be unittests for this as well.

 

or the worker/runworker code.

What would you suggest these tests look like? They can't spin up a worker and fire requests at it, and most of the worker code is interactive.

It is certainly possible to fire up a worker in a subprocess and test it that way. Or you could create an instance of the Worker object and call its methods.

 

 

The changes to runserver aren't tested.

Runserver in Django only has a single test, presumably for the same reason; I don't know how I can write a test for this considering the autoreload and extra process mechanisms we'd want to cover.

 

The new test utilities aren't tested either (yes that's a thing).

I would say that the usage of those utilities in other tests means they are tested sufficiently, given that they're used sufficiently; writing a test for just the channel test case would look very similar to the basic tests elsewhere.

The existing test utilities (test client, asserting the number of queries, overriding settings, etc) have tests of their own https://github.com/django/django/blob/master/tests/test_utils/tests.py as do the extensions added by contrib.staticfiles. I don't see why Channels is special and therefore doesn't need them.

 

 

All new code should have tests. Isn't that Django's current standard?

There's no clear standard, the docs just say the tests should "exercise all of the new features". I admit that auth/session needs tests - I'll get those done this week - but I don't see how we can reasonably test the interactive commands in an automated fashion given the current test harness (and lack of tests for things like runserver) in Django.

To me it's a bad code smell when code is hard to test. I can't believe I'm having to convince anyone in this community about the value of testing.

 

 

asgiref has one "basic" test around WSGI handling which the usage is documented in this change. I don't think anyone would dare say that covers potential edge cases in WSGI/HTTP handling. Not test related, the status code map is also missing many HTTP status codes.

The WSGI handling part of asgiref is not considered complete, and I will probably remove the section about using it in the main Django docs. Only the inmemory backend should be considered used by Django.

 

This is exactly my point. This project is not complete. That is not at all clear from this PR or from looking at the asgiref README which only re-enforces my feeling that this dependency should not be a requirement when installing Django.

[Andrew Godwin]

On Wed, May 4, 2016 at 11:52 AM, Mark Lavin <markd...@gmail.com> wrote:

Given that there is no guarantee of message delivery, I don't think this is suitable or recommended for using for background tasks. I don't think the Django docs should encourage that usage. At least without a strong warning that this isn't a good idea.

 

There are tests for the routing and request handling but there are no unittests that I can see for the auth/session handling,

Yes, these are missing.

 

Channel/Message/Group/ChannelLayerManager/custom JSON encoder classes and their usage,

The ASGI conformance test suite takes care of this: https://github.com/andrewgodwin/django/blob/channels/tests/channels_tests/test_database_layer.py#L3 (that's also used for the inmemory module and asgi_redis in their test suites)

 

Yes the code is covered by integration/functional tests but again I feel like there should be unittests for this as well.

Those are unit tests as far as I'm concerned, and they're quite comprehensive - there's no smaller units of work to really write tests _for_ in any of these backends.

 

 

or the worker/runworker code.

What would you suggest these tests look like? They can't spin up a worker and fire requests at it, and most of the worker code is interactive.

It is certainly possible to fire up a worker in a subprocess and test it that way. Or you could create an instance of the Worker object and call its methods.

I'll look into this, but firing up subprocesses won't work as it suddenly introduces Redis as a test dependency (right now all tests use the in-memory backend).

 

 

 

The changes to runserver aren't tested.

Runserver in Django only has a single test, presumably for the same reason; I don't know how I can write a test for this considering the autoreload and extra process mechanisms we'd want to cover.

 

The new test utilities aren't tested either (yes that's a thing).

I would say that the usage of those utilities in other tests means they are tested sufficiently, given that they're used sufficiently; writing a test for just the channel test case would look very similar to the basic tests elsewhere.

The existing test utilities (test client, asserting the number of queries, overriding settings, etc) have tests of their own https://github.com/django/django/blob/master/tests/test_utils/tests.py as do the extensions added by contrib.staticfiles. I don't see why Channels is special and therefore doesn't need them.

I would counter that by saying that the Channels test handler code is only about 20 additional lines of logic and no new assertions, so the existing suite is more than enough.

 

 

 

All new code should have tests. Isn't that Django's current standard?

There's no clear standard, the docs just say the tests should "exercise all of the new features". I admit that auth/session needs tests - I'll get those done this week - but I don't see how we can reasonably test the interactive commands in an automated fashion given the current test harness (and lack of tests for things like runserver) in Django.

To me it's a bad code smell when code is hard to test. I can't believe I'm having to convince anyone in this community about the value of testing.

Well, it is hard to test, mostly due to the fact that testing websockets requires the test client to also be asynchronous as well as the server, and if they're in the same process that leads to trouble.

I am very much aware of the value of testing, but I'm saying that in this case it's a large hurdle to climb, I believe the tradeoff is acceptable to merge it in for the alpha as it is and then get the MOSS program to pay some people to work on just this part so it gets the right level of attention.

 

 

 

asgiref has one "basic" test around WSGI handling which the usage is documented in this change. I don't think anyone would dare say that covers potential edge cases in WSGI/HTTP handling. Not test related, the status code map is also missing many HTTP status codes.

The WSGI handling part of asgiref is not considered complete, and I will probably remove the section about using it in the main Django docs. Only the inmemory backend should be considered used by Django.

 

This is exactly my point. This project is not complete. That is not at all clear from this PR or from looking at the asgiref README which only re-enforces my feeling that this dependency should not be a requirement when installing Django.

Of course the project isn't complete, if it was I wouldn't be trying to merge it in for an alpha; would you feel happier if I upped asgiref to version 1.0 and stuck a comment on the wsgi part saying it's not done?

Please appreciate that I've spent a hell of a lot of time getting the project to this point, and anything missing is not because of malice or incompetence but because I don't believe that 100% test coverage is a necessary prerequisite for getting something merged in, merely sufficient test coverage, and I'm trying to strike a balance between getting this actually done in my spare time and getting it landed in 1.10.

Sessions and auth were definitely in need of tests (I just hadn't found time yet), so I've pushed up a session test suite today, and I'll look at auth later on. I think the rest, however, is sufficiently in line with the way Django currently is tested that we should merge it in and work on tests for the new (and old!) runserver during the run-up to the release - we have the funding to pay people to do this, it's just taken longer to get everything set up to pay people than I would have liked.

Andrew

[Mark Lavin]

I like Markus' suggestion and I think that's in line with how Django handles other optional dependencies such as the db bindings (psycopg2, MySQLdb, etc). Those raise an ImproperlyConfigured exception when there is an import error. The memcache cache backend on the other hand raises an import error if python-memcached isn't installed.

[Mark Lavin]

I can (and do) appreciate the effort that's gone into this work while still feeling as though it isn't ready to be merged with Django. To be honest given that this PR contains almost no changes to Django itself and only adds new code, I don't understand why it can't live outside of Django while it continues to mature.

Best,

Mark

On Wednesday, May 4, 2016 at 3:18:03 PM UTC-4, Andrew Godwin wrote:

On Wed, May 4, 2016 at 11:52 AM, Mark Lavin <markd...@gmail.com> wrote:

Given that there is no guarantee of message delivery, I don't think this is suitable or recommended for using for background tasks. I don't think the Django docs should encourage that usage. At least without a strong warning that this isn't a good idea.

 

There are tests for the routing and request handling but there are no unittests that I can see for the auth/session handling,

Yes, these are missing.

 

Channel/Message/Group/ChannelLayerManager/custom JSON encoder classes and their usage,

The ASGI conformance test suite takes care of this: https://github.com/andrewgodwin/django/blob/channels/tests/channels_tests/test_database_layer.py#L3 (that's also used for the inmemory module and asgi_redis in their test suites)

 

Yes the code is covered by integration/functional tests but again I feel like there should be unittests for this as well.

Those are unit tests as far as I'm concerned, and they're quite comprehensive - there's no smaller units of work to really write tests _for_ in any of these backends.

 

 

or the worker/runworker code.

What would you suggest these tests look like? They can't spin up a worker and fire requests at it, and most of the worker code is interactive.

It is certainly possible to fire up a worker in a subprocess and test it that way. Or you could create an instance of the Worker object and call its methods.

I'll look into this, but firing up subprocesses won't work as it suddenly introduces Redis as a test dependency (right now all tests use the in-memory backend).

 

 

 

The changes to runserver aren't tested.

Runserver in Django only has a single test, presumably for the same reason; I don't know how I can write a test for this considering the autoreload and extra process mechanisms we'd want to cover.

 

The new test utilities aren't tested either (yes that's a thing).

I would say that the usage of those utilities in other tests means they are tested sufficiently, given that they're used sufficiently; writing a test for just the channel test case would look very similar to the basic tests elsewhere.

The existing test utilities (test client, asserting the number of queries, overriding settings, etc) have tests of their own https://github.com/django/django/blob/master/tests/test_utils/tests.py as do the extensions added by contrib.staticfiles. I don't see why Channels is special and therefore doesn't need them.

I would counter that by saying that the Channels test handler code is only about 20 additional lines of logic and no new assertions, so the existing suite is more than enough.

 

 

 

All new code should have tests. Isn't that Django's current standard?

There's no clear standard, the docs just say the tests should "exercise all of the new features". I admit that auth/session needs tests - I'll get those done this week - but I don't see how we can reasonably test the interactive commands in an automated fashion given the current test harness (and lack of tests for things like runserver) in Django.

To me it's a bad code smell when code is hard to test. I can't believe I'm having to convince anyone in this community about the value of testing.

Well, it is hard to test, mostly due to the fact that testing websockets requires the test client to also be asynchronous as well as the server, and if they're in the same process that leads to trouble.

I am very much aware of the value of testing, but I'm saying that in this case it's a large hurdle to climb, I believe the tradeoff is acceptable to merge it in for the alpha as it is and then get the MOSS program to pay some people to work on just this part so it gets the right level of attention.

 

 

 

appreciate

asgiref has one "basic" test around WSGI handling which the usage is documented in this change. I don't think anyone would dare say that covers potential edge cases in WSGI/HTTP handling. Not test related, the status code map is also missing many HTTP status codes.

The WSGI handling part of asgiref is not considered complete, and I will probably remove the section about using it in the main Django docs. Only the inmemory backend should be considered used by Django.

This is exactly my point. This project is not complete. That is not at all clear from this PR or from looking at the asgiref README which only re-enforces my feeling that this dependency should not be a requirement when installing Django.

[Andrew Godwin]

On Wed, May 4, 2016 at 12:28 PM, Mark Lavin <markd...@gmail.com> wrote:

I can (and do) appreciate the effort that's gone into this work while still feeling as though it isn't ready to be merged with Django. To be honest given that this PR contains almost no changes to Django itself and only adds new code, I don't understand why it can't live outside of Django while it continues to mature.

That's a perfectly valid question, and one I've debated a lot. It's mostly because channels is meant to be a fundamental change to Django (inserting a new layer below views and middleware), and having it as something officially supported and maintained is far better than having a third-party package everyone basically relies on but which doesn't benefit from the support of a large team (having been there with South, I know I don't want to go back).

It's even more difficult to argue considering it does exist as a perfectly functional third-party package for 1.8/1.9, but the goal with channels is, for me, to move Django along in the world of web technology and make sure it's positioned right for what the job of a webserver is becoming, which as far as I am concerned inexorably involves websockets and other "async" things, and I think having it in Django itself sends the right message.

Most of the core team I have spoken to about this seem to agree with me, which is why I'm pushing so hard. Obviously that trust and maintenance that you get by being in core Django comes at a price, and the price is having good code in the first place, but I believe that the code is very close to being good enough considering we're labelling the whole feature as "provisional" in 1.10 anyway (which is something Python recently started doing for similar not-experimental but not-100%-final APIs)

(Also, I would posit that the fact it barely changes anything in Django is part of the good design, but I am biased there!)

Andrew 

[Marc Tamlyn]

Major features merged into Django have generally never been as "perfect" as the standards required for smaller patches. There's a recognisation of the need for ongoing work, probably over the course of multiple versions, in order to perfect any major new feature. The effort involved in getting a patch like this to the point it can be merged at all is very significant. Many major patches (composite fields, templates widgets...) have previous not landed, sometimes multiple times, because they are nowhere near the standard that this channels patch is.

My feeling is that the core team agree with Andrew that this is an important direction for Django, and 1.10 is the right release to include it in. Bug fixes, additional tests and so on can all be added between alpha and final as needed. The important thing is that we have broad agreement that the feature is good, the design is right, and an understanding that shortcomings will be worked on over the next year or two.

Marc

--

You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAFwN1uqpLNTLDuZzNQzH_4k_ZYzHx0pbuXzeKo7u80MjVMr3HA%40mail.gmail.com.

[Jacob Kaplan-Moss]

On Wed, May 4, 2016 at 2:45 PM, Marc Tamlyn <marc....@gmail.com> wrote:

Major features merged into Django have generally never been as "perfect" as the standards required for smaller patches. There's a recognisation of the need for ongoing work, probably over the course of multiple versions, in order to perfect any major new feature. The effort involved in getting a patch like this to the point it can be merged at all is very significant. Many major patches (composite fields, templates widgets...) have previous not landed, sometimes multiple times, because they are nowhere near the standard that this channels patch is.

My feeling is that the core team agree with Andrew that this is an important direction for Django, and 1.10 is the right release to include it in. Bug fixes, additional tests and so on can all be added between alpha and final as needed. The important thing is that we have broad agreement that the feature is good, the design is right, and an understanding that shortcomings will be worked on over the next year or two.

I agree with this completely. I've been using Channels semi-seriously for 2-3 months now, and in my opinion it easily clears the bar for inclusion. The design is right, it's stable in production and under load, and has no show-stopping bugs I've been able to find. Quite bluntly, it's probably somewhat better than our average alpha-quality new feature. 

My only real concern was around performance regressions, but now that the WSGI "mode" hits no new code paths, that's no longer a concern. 

Jacob 

[Mark Lavin]

Major features have never been perfect, no, but they have in the past typically gone through two paths to prove out their design/API/usefulness. One is as an established and mature third-party app such as messages, staticfiles, and django-secure. More recently the other has been through the DEP process: multiple templates (Jinja) and query expressions. Channels has done neither.

Sorry if it seems that I've raised these issues late but I don't feel like there has been a good place for this discussion since the DEP process was circumvented. Most of the development for this has been in Andrew's space. I don't feel welcome to raise a dissenting opinion as a mere lowly member of the Django community. It's pointless for me to continue to elaborate on the things I don't like about channels as I'm clearly in the minority and it's going to land. All I can do now is beg for these requirements to be optional.

- Mark

[Tim Graham]

The future is hard to predict, but I'll add that I'm a tad nervous about this as well.

I'm completely inexperienced with channels at this time. Who else has a good grasp of the code right now and could help fix release blocking bugs if Andrew isn't available? Are we playing for any or all bug fixes? If these requests have to go through the MOSS committee, will that delay getting things fixed?

Migrations delayed the 1.7 release for months, and I'd like some vote of confidence that merging this at such a late stage won't cause similar delays.

I'll include the release schedule here as a reminder:

May 16	Django 1.10 alpha; feature freeze.
June 17	Django 1.10 beta; non-release blocking bug fix freeze.
July 15	Django 1.10 RC 1; translation string freeze.
2+ weeks after RC1	Django 1.10 final (or RC 2, if needed).

[Andrew Godwin]

On Wed, May 4, 2016 at 6:12 PM, Tim Graham <timog...@gmail.com> wrote:

The future is hard to predict, but I'll add that I'm a tad nervous about this as well.

I'm completely inexperienced with channels at this time. Who else has a good grasp of the code right now and could help fix release blocking bugs if Andrew isn't available? Are we playing for any or all bug fixes? If these requests have to go through the MOSS committee, will that delay getting things fixed?

Migrations delayed the 1.7 release for months, and I'd like some vote of confidence that merging this at such a late stage won't cause similar delays.

I can't speak to everything here, but I do plan to be available through the course of the release and some time after; that's not ideal from a bus factor point of view, but it's better than me knowing I'm not around I guess (unlike migrations, where I was moving continent while it was landing).

Channels is a simpler patch than migrations IMO, but even labelling it as "provisional" doesn't get around the fact we need to prevent bugs, it just saves us in the future if we need to change the API a bit. I have a high confidence level in it - especially as nearly all of the functionality is additional, so there's a very low risk of breaking existing Django projects - but some risk remains.

If you want we could take this to the technical board and get it voted on (a vote from which I will abstain, since I'm on the same board), which would seem to be the Right Thing to do given the concerns being raised.

Andrew

[Russell Keith-Magee]

Hi Mark,

On Thu, May 5, 2016 at 8:41 AM, Mark Lavin <markd...@gmail.com> wrote:

Major features have never been perfect, no, but they have in the past typically gone through two paths to prove out their design/API/usefulness. One is as an established and mature third-party app such as messages, staticfiles, and django-secure. More recently the other has been through the DEP process: multiple templates (Jinja) and query expressions. Channels has done neither.

Sorry if it seems that I've raised these issues late but I don't feel like there has been a good place for this discussion since the DEP process was circumvented. Most of the development for this has been in Andrew's space. I don't feel welcome to raise a dissenting opinion as a mere lowly member of the Django community.

If that’s your perception, then we as a community clearly have a problem that needs to be addressed. 

You’ve been around the Django community since (AFAICT) 2009. You’re a technical director at a well known and well respected Django consultancy. You’ve given talks at DjangoCons. You’ve co-written a book about Django for O’Reilly. If you’re not someone who is in a position to give an informed opinion on issues with Channels, then I don’t know who is. If you feel like you’re on the outer and your opinion is not welcome, then that’s *our* failure, not yours.

I can’t argue with the fact that the DEP process has been circumvented here. I also acknowledge that this would be doubly frustrating given your difficulties shepherding the content negotiation DEP. I don’t think I can give a good answer for why this has been done, other than enthusiasm and momentum overriding a not-entirely-well-established process.

This thread (and email in general) probably isn’t the best place to flesh out the solution to these process issues, but they definitely need to be resolved. Discussion at PyCon and DjangoCon US is definitely called for - I’ll be at both, and I’d definitely be interested in a BOF or similar session to field opinions and thoughts from the community about this process.

 

It's pointless for me to continue to elaborate on the things I don't like about channels as I'm clearly in the minority and it's going to land. All I can do now is beg for these requirements to be optional.

Talking about channels specifically: This shouldn’t be true at all. There’s certainly some momentum, but the code isn’t in trunk, so it’s not too late. Even if it *was* in trunk - if you could demonstrate that there was a serious problem, I’d support removing it from the codebase.

I will admin that I haven’t been paying *close* attention to Andrew’s work - I’m aware of the broad strokes, and I’ve skimmed some of the design discussions, but I haven’t been keeping close tabs on things. From that (admittedly weak) position, the only counterargument that I’ve heard are performance concerns. 

However, the last thing I heard on this subject was that the “raw WSGI” path was essentially unmodified, so there shouldn’t be any particular performance concerns from adding this to the codebase - just new functionality for targeting websockets. For me, websockets are a big area where Django is currently lacking, and everything I’ve read about Andrew’s proposal has struck me as a reasonable compromise between Django’s need to respond to a technical requirement, while maintaining ease of implementation. 

I take it that this isn’t your opinion. If you’ve got other concerns - be they specific implementation issues, or broader philosophical issues, I’d strongly encourage you to express them. The core team and/or technical board might not agree with you, but we’d be fools to ignore your experience and opinions. At the absolute minimum, you’ve earned a considered response to your concerns.

Yours,

Russ Magee %-)

[Mark Lavin]

Thank you Russ. I'll reconsider expressing my full thoughts on Channels more likely in another thread. For now I do think it's worth addressing this issue of benchmarks/performance which keeps being brought up. The argument is that since this is optional we don't need to see the benchmarks because there won't be any regressions, which is true. However, if it is also being said that this is so fundamentally important to Django and everyone will use it so it cannot live as an external project and must land in 1.10 then I don't think that argument can be made without ensuring there are no huge regressions moving an existing application from WSGI to ASGI. If nothing else those benchmarks seem important for Django users to make an informed choice about WSGI vs ASGI for their deployment. How can we not care about how this "fundamental change to Django" might impact performance or say that isn't a requirement to even measure, regardless of outcome, before its inclusion?

- Mark

[Aymeric Augustin]

FWIW I’m in the same boat as Russell:

- limited familiarity with channels: I read the docs cover-to-cover but never ran the code

- sufficient trust in their design: I heard Andrew talk about it and I thought it made sense

- reasonable confidence that it won’t introduce regressions, including performance regressions (at least with the in-memory backend)

Regarding risks for the 1.10 release, I don’t expect channels to be as problematic as migrations, if only because they’re entirely determined by the current version of the code. In contrast, migrations could be affected by all previous versions of the code, adding a whole new dimension of complexity.

Regarding the process, I’m familiar with the situation :-( When I worked on transactions and app-loading, I argued heavily and merged large backwards-incompatible patches at interim steps, just to keep things manageable, under the assumption that I’d manage to figure out a consistent design by the next release. That turned out OK but it was less than satisfying.

I tried to improve by getting funding for the “multiple template engines”. A secret goal of that project was to establish a standard of public accountability for funded projects. I received positive feedback on the method — detailed DEP, weekly updates — and I’d love if others adopted it. The topic was nowhere nearly as ambitious as channels, though.

Finally I’m interested in hearing more about the “things [Mark doesn’t] like about channels”. Channels has been mostly a solo effort by Andrew. However, until now, public discussion has usually reinforced by trust in his design. We'll see if that trend holds ;-)

-- 

Aymeric.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/455347ad-689c-49c2-a1c0-ace3127a3583%40googlegroups.com.

[Anssi Kääriäinen]

On Thursday, May 5, 2016, Russell Keith-Magee <rus...@keith-magee.com> wrote:

I will admin that I haven’t been paying *close* attention to Andrew’s work - I’m aware of the broad strokes, and I’ve skimmed some of the design discussions, but I haven’t been keeping close tabs on things. From that (admittedly weak) position, the only counterargument that I’ve heard are performance concerns. 

I haven't brought this up before, but security is something we should discuss pre-merge.

What I'm mainly worried about is malicious clients intentionally trying to choke the channels layer. I guess the approaches for DoS attack would fall under these categories:

  1. Try to generate large responses and read those response slowly.

  2. Fire a large request, don't read the response.

  3. Try to cause exceptions in various parts of the stack - if the worker never writes to the response channel, what will happen?

There are always DoS vectors, but checking there aren't easy ones should be done. The main concern with channels is potential resource leak.

I found accidentally some presentations that seem relevant to thus discussion. I recently watched some presentations about high availability at Braintree. There are two presentations available, one from 2013 by Paul Gross, where he explains their approach to HA, and one from 2015 by Lionel Barrow, explaining what changed. Both are very interesting and highly recommended.

The 2013 presentation introduces one key piece to HA at braintree, called Broxy. Broxy basically serves HTTP the same way as Daphne - write requests to tedis, and wait for response again through Redis.

The 2015 representation explains what changed. They removed Broxy because it turned out to be conceptually complex and fragile. It might be their implementation. But there is certain level of both complexity and possible fragility about the design. On the other hand their story pretty much verifies that the design does scale.

All in all I'd feel a lot more confident if there were large production sites using the channels system, so that we shouldn't theorize on the excellence of the approach. Are there already production deployments out there?

 - Anssi

 

[Andrew Godwin]

On Thu, May 5, 2016 at 12:55 AM, Anssi Kääriäinen <akaa...@gmail.com> wrote:

On Thursday, May 5, 2016, Russell Keith-Magee <rus...@keith-magee.com> wrote:

I will admin that I haven’t been paying *close* attention to Andrew’s work - I’m aware of the broad strokes, and I’ve skimmed some of the design discussions, but I haven’t been keeping close tabs on things. From that (admittedly weak) position, the only counterargument that I’ve heard are performance concerns. 

I haven't brought this up before, but security is something we should discuss pre-merge.

What I'm mainly worried about is malicious clients intentionally trying to choke the channels layer. I guess the approaches for DoS attack would fall under these categories:

  1. Try to generate large responses and read those response slowly.

This would likely lead to either the response packets expiring in redis after one minute, or redis running out of memory as it overfills with packets and doing whatever its configured OOM response is (which I will suggest be "expire things early"). asgi_redis could likely be improved so that the channel lists don't get overfull in this situation.

 

  2. Fire a large request, don't read the response.

Same as above. ASGI has no backpressure on channels per se, so you can't tell if the response is being read at all.

 

  3. Try to cause exceptions in various parts of the stack - if the worker never writes to the response channel, what will happen?

Daphne will time out the request after 120 seconds from request start with a 503 Service Unavailable in the default configuration, but I'm tempted to drop that to 60 and reset the clock every time a response chunk turns up, and have a second absolute timeout that handles the slow-reader DoS case.

 

There are always DoS vectors, but checking there aren't easy ones should be done. The main concern with channels is potential resource leak.

I found accidentally some presentations that seem relevant to thus discussion. I recently watched some presentations about high availability at Braintree. There are two presentations available, one from 2013 by Paul Gross, where he explains their approach to HA, and one from 2015 by Lionel Barrow, explaining what changed. Both are very interesting and highly recommended.

The 2013 presentation introduces one key piece to HA at braintree, called Broxy. Broxy basically serves HTTP the same way as Daphne - write requests to tedis, and wait for response again through Redis.

The 2015 representation explains what changed. They removed Broxy because it turned out to be conceptually complex and fragile. It might be their implementation. But there is certain level of both complexity and possible fragility about the design. On the other hand their story pretty much verifies that the design does scale.

Do you have a link to the presentation about them removing it? I've tried to solve the problems I had last time I wrote a reverse proxy like this by making the thing drop requests and responses whenever a problem comes up (previous ones I've worked with suffered from a bad recovery after high traffic as they tried to get through the queue)

The main problem with the design that I can potentially forsee is the lack of backpressure, which was raised before. While it keeps the design simple, it also means that anyone writing into a channel has no idea about the current state of it; the problem is, however, that what constitutes "full" varies by channel (e.g. for http.request it could be 1000 packets, for http.response.clientid it could be 10), and so you start needing per-channel, per-project configuration.

I'm tempted to add this as a configurable option on the channel layer (defaulting every channel to, say, 500), though, and extend the ASGI spec just slightly to allow send() to return a ChannelFull exception, which clients can treat how they like (daphne might drop the request, django might just wait while writing a response and give up after X seconds).

 

All in all I'd feel a lot more confident if there were large production sites using the channels system, so that we shouldn't theorize on the excellence of the approach. Are there already production deployments out there?

Jacob mentioned he'd been running it on something, but I don't know what exactly, and I only hear mention, not details, of people trying it in production. Personally, it's only running on my own sites, which are anything but high traffic.

I had discussed with several people before about getting Channels running as a parallel to their site and redirecting some load to it via a hidden iframe to do a slightly better load test; maybe now is the time to start that process?

Andrew 

[Anssi Kääriäinen]

On Thursday, May 5, 2016, Andrew Godwin <and...@aeracode.org> wrote:

Do you have a link to the presentation about them removing it?

https://youtu.be/839rskyJaro around 34 minutes and onwards, another version is https://youtu.be/2dG5HeM7MvA at 24 minutes.

They were tackling a bit different problem, so their lessons might not apply. Most importantly they weren't aiming for websockets at all, just for high availability and throughput for normal HTTP traffic. On the other hand, the architecture of broxy itself is pretty much the same as Daphne, and they felt they had problems with it.

The way we have approached recent feature additions is that we let them prove themselves outside of core. I think the crucial question is why Channels can't do this, that is why can't we wait some time and let actual production deployments prove the design?

I know South is an example of why some features have a hard time living outside core. But DRF is an example that a feature crucial to modern web development can work very well outside of core.

 - Anssi

[Andrew Godwin]

On Thu, May 5, 2016 at 9:28 PM, Anssi Kääriäinen <akaa...@gmail.com> wrote:

On Thursday, May 5, 2016, Andrew Godwin <and...@aeracode.org> wrote:

Do you have a link to the presentation about them removing it?

https://youtu.be/839rskyJaro around 34 minutes and onwards, another version is https://youtu.be/2dG5HeM7MvA at 24 minutes.

Summarising their issues here for future people to save on video watching:

- Large response bodies/streamed/chunked bodies are hard to serialise into Redis

- Working out the order to restart things in is tricker than standard HTTP loadbalancing

- Healthchecking the Rails process was difficult as they didn't listen on HTTP (just to Redis)

- Hard to do rate-limiting and monitoring as it lacked context

- HAProxy got a lot more featureful and grew the features they needed (hot-swapping of backends), and was a lot more battle-tested

In general, it seems they were mainly using it to achieve HA, and it didn't quite pull through for them on that front. Some of these issues also apply to Channels, some less so as it's not designed as a HA solution and just being used for that.

In particular, they were doing a lot more to keep requests around, whereas Channels will drop them if it's unsure what's going on, which gives it a lot more leeway (but makes it less "perfect" at HA).

 

They were tackling a bit different problem, so their lessons might not apply. Most importantly they weren't aiming for websockets at all, just for high availability and throughput for normal HTTP traffic. On the other hand, the architecture of broxy itself is pretty much the same as Daphne, and they felt they had problems with it.

I need to start writing up my preferred large-scale deployment architecture for Channels more, which is "small clusters of interfaces and workers with an external loadbalancer across all clusters"; all of the stories I've heard, this included, the common theme is trying to push all of the site traffic through just the one bus and having great issues when it keels over under unexpected load or machine failure.

I'm also starting to think we should have a local shared-memory channel layer that only works between processes on the one machine, so one could in theory just run clusters like this on decently sized multicore boxes; that would make a lot of people happier who don't want to have to run Redis in production but still want the multi-process behaviour that Channels gives. Running Channels would then just mean two processes, Daphne and workers.

 

The way we have approached recent feature additions is that we let them prove themselves outside of core. I think the crucial question is why Channels can't do this, that is why can't we wait some time and let actual production deployments prove the design?

I know South is an example of why some features have a hard time living outside core. But DRF is an example that a feature crucial to modern web development can work very well outside of core.

Channels sits somewhere between these two; it's not quite as deeply involved in hacking around core parts of Django as South was, but it's a bit deeper than DRF gets since it technically inserts itself under the entire view/URL routing system and sits alongside the WSGI handling code.

My reason for pushing to get it in is that a lot of the utility of Channels is not in the code itself but what it enables; most of the things I would like to build can sit on top as third-party packages and get pulled in later if they make sense (e.g. a nice set of generic websocket consumers, or a high-level model-changes streaming solution).

If it lives as some third-party package for another 8 months, it's a lot harder to sound that rally cry to build up the necessary community of layers on top of it that complete the ecosystem; plus, there's the obvious human factor that it will likely place the same workload on a much smaller set of main contributors.

The situation is helped by the fact that the code that will likely need most polish and iteration - Daphne and asgi_redis - will still live outside Django's core codebase, giving us some more flexibility in how we handle changes (though I would like to still provide similar levels of stability and security assurances on them) - and that we'll be labelling it as "provisional" in 1.10 (similar to how PEP 411 describes it for Python), meaning we get one last chance to tweak things in 1.11 before we lock the API in for good.

Andrew

[Tim Graham]

I'm not convinced either way about whether putting this in core will help mature it and fix bugs more quickly or not. I don't have any sense of how the code might change after we merge it, but things get more complicated if we start selectively backporting somes fixes for Django's monthly bug fix releases. If channels continues to live externally, I assume there won't be the complication of a master vs. stable branch and that releases could happen more often. If this feature is exciting enough, whether or not its in Django itself shouldn't make too much of a difference, should it? Also, if we do include this in 1.10 and then start paying people using the Mozilla money, that might result in master changing even more aggressively compared to the stable branch at which point having a 1.10 release that's X months old in terms of the latest features might not be all that valuable.

Tell me if I'm wrong, but I get the sense that you are somewhat burned out maintaining this project on your own and you feel that merging this to Django will help offload your burden and attract other contributors. If that's the case, there's a possible solution in using some of the Mozilla money to try to get help in more of the code review/maintenance tasks. If we have an accepted DEP and plans to merge it to core in 1.11, I would try to get more involved with those projects too.

I don't see a problem in continuing to refine this externally even if that means it lands in an LTS. I sure would feel less overwhelmed if I didn't have to review the big patch up against the alpha deadline and/or if this threatens to delay the 1.10 release.

[Andrew Godwin]

On Mon, May 9, 2016 at 6:42 PM, Tim Graham <timog...@gmail.com> wrote:

I'm not convinced either way about whether putting this in core will help mature it and fix bugs more quickly or not. I don't have any sense of how the code might change after we merge it, but things get more complicated if we start selectively backporting somes fixes for Django's monthly bug fix releases. If channels continues to live externally, I assume there won't be the complication of a master vs. stable branch and that releases could happen more often. If this feature is exciting enough, whether or not its in Django itself shouldn't make too much of a difference, should it? Also, if we do include this in 1.10 and then start paying people using the Mozilla money, that might result in master changing even more aggressively compared to the stable branch at which point having a 1.10 release that's X months old in terms of the latest features might not be all that valuable.

A lot of the future work is not on the core code itself but additional code around it; this was the whole point of getting a core amount of it into 1.10, so that there was something consistent to build around.

 

Tell me if I'm wrong, but I get the sense that you are somewhat burned out maintaining this project on your own and you feel that merging this to Django will help offload your burden and attract other contributors. If that's the case, there's a possible solution in using some of the Mozilla money to try to get help in more of the code review/maintenance tasks. If we have an accepted DEP and plans to merge it to core in 1.11, I would try to get more involved with those projects too.

I'm not burned out maintaining it - I'm burned out trying to argue about getting it into core, so I'm inclined to stop arguing for that reason alone, and instead revert to the tactic I know best of running huge features out of a slightly-monkeypatchy external package :)

My concern around using the Mozilla money to refine it was that it was applied for under the pretense this would be core Django, though if that's still our intention and we keep it external for now I don't see too much of a problem arising there.

 

I don't see a problem in continuing to refine this externally even if that means it lands in an LTS. I sure would feel less overwhelmed if I didn't have to review the big patch up against the alpha deadline and/or if this threatens to delay the 1.10 release.

I understand, and I'm sorry for not getting this done in a more timely fashion; I should have had the patch ready earlier than it was, but didn't get round to it for a while, and (mistakenly) thought a few weeks would be enough.

If we do keep it external, I would suggest that we still move to adopt it as an official Django project, moving ownership of the git repos and mentioning it in official documentation; this to me seems like a less severe thing than merging it into core, and also something that there is not a deadline preventing so we can discuss it further if needs be.

Andrew

[Anssi Kääriäinen]

On Tue, May 10, 2016 at 4:54 AM, Andrew Godwin <and...@aeracode.org> wrote:
> My concern around using the Mozilla money to refine it was that it was
> applied for under the pretense this would be core Django, though if that's
> still our intention and we keep it external for now I don't see too much of
> a problem arising there.

I don't see a problem here - the main opposition seems to be about
having the code in Django 1.10. This puts a lot of pressure on Tim,
and there isn't enough time to field-test the design. It also feels
wrong that we approach many feature additions with saying that the
design should be proven in a 3rd party app, but there hasn't yet been
enough time to prove the channels design.

Tim's feeling about this is very important to me. He is doing an
awesome job reviewing and committing patches and ensuring releases
happen on time. In my opinion he should have control of what happens
near feature freeze. If he doesn't have that control, it will make it
impossible for him to ensure we stay on release schedule.

I'm going to try to avoid writing more about this. My opinion is
hopefully clear enough already. If the decision is to ship this in
1.10, then the available time should be used to make this feature as
good as possible, not on this discussion.

- Anssi

[Tom Christie]

My preference would be to shift the alpha deadline *without* yet making a firm decision on if channels hits 1.10 or not. That would take a little pressure off, and not force anyone into making a decision prematurely.

Moving the window back (say 3 weeks?) and allowing a little more time for the DEP and design discussion around it, sounds preferable to making a rushed judgement in either direction.

Cheers

  - Tom

[Yo-Yo Ma]

To hopefully add to this conversation:

I'll start by saying I've enjoyed the contributions by Andrew over the years and believe he is a most excellent developer.

A couple months ago, around the same time that JKM started Andrew's repo (which is what got my attention) I decided to give Channels a try. I had a quick honeymoon with it because I had been working on a game and was using aiohttp. aiohttp provides support for websockets but doesn't manage relationships between them, which makes it hard to do anything without building a layer in between your app and the web. This layer ends up being somethings like Channels. Channels basically accomplished what I needed, which I was ecstatic about.

The honeymoon ended when I started using Channels to experiment on my local machine. The notion that I would now have to use a data store to run my app at all didn't feel right, even though I knew there would need to be data stored to maintain relationships between websockets, etc. I was disheartened when I learned Channels was potentially going to be merged into Django, because I finally got to that realization of the fact that there wasn't a "right way" to include baked-in web sockets behind a web server, only to find out that Channels was going to be called the "right way", forever (potentially).

So, let me ask a question: how many of the past 5 paid projects you worked on required websockets? For me, and for most of the people I know (not many), the answer is 0. I can think of a use case where it would have been nice (the "how many other users are viewing this page" feature, which we accomplished with slow Ajax polling), but I can think of nothing critical.

I ask that question because a good argument came up earlier, a comparison to NoSQL. NoSQL was in the same must-have craze, and then the tried and true standard in Django (Postgres) sort of answered that with huge gains in performance and scalability. Competition is great for this reason, but merging in competion is sort of like a business acquiring innovative. competitors as they emerge; it doesn't do much to further the company's true vision, because it's a defensive move.

I would recommend against merging Channels now, and potentially ever; not because it's not great code, but because A) it isn't necessary for the core, B) leaving it out of the core doesn't prevent it from being used, and C) it should be proven over time as an outside package (a different way of solving this could come by and proliferate while we have Channels in core, and then Django would be heading back down that hill towards the bloat (e.g., contrib) it worked so hard to remove over the years - not that Django was ever *truly* bloated).

What I would recommend is that we focus on fixing/abstracting/documenting things that make it hard to integrate something like Channels (e.g., things that Andrew has to monkey patch, etc.), because Channels *and* a hypothetical future websockets package could both take advantage of such an abstraction layer.

Political tirade follows:

Another thing I would never recommend is letting a grant destroy Django's ability to make a decision on its own. It absolutely disgusts me to think of Django becoming like Firefox, with "features" like Pocket and Hello that are driven by financial or political desires. Django should continue to look unemotionally at feature requests without regard for sunk cost fallacy (e.g., "<developer> already spent X time working on this...", etc.).

[Yo-Yo Ma]

Correction:

*JKM starred (not started) - stupid, stupid iPhone.

[Tom Christie]

> The notion that I would now have to use a data store to run my app at all didn't feel right

Fortunately, you don't need to. From the docs in the pull request... "It's an optional part of Django; you don't need to interact with it if you're just writing a normal website, but if you want the features, you can just turn it on."

This isn't comparable to third-party packages, because it's a foundational aspect of the framework. Channels making its way into core is advantageous in a way that isn't nearly as true for other kinds of features.

> Political tirade follows

I don't think that discussion is likely to be a productive route to go down. Everyone here is motivated by the same desire to make Django be the best thing it can be. :)