Discussion:
Django Integration
Chad Paulson
2016-03-04 23:58:16 UTC
Permalink
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/
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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.
Daniel Chimeno
2016-03-05 12:05:48 UTC
Permalink
Hello,

There are more information about the project in the doc page:
https://channels.readthedocs.org/en/latest/
Post by 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/
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/f8106391-7ee4-4c21-9f4c-12fc342dbc21%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Andrew Godwin
2016-03-05 19:39:32 UTC
Permalink
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
Post by 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/
--
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
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
<https://groups.google.com/d/msgid/django-developers/2bdf074c-5e21-48cf-a93c-ba19f7744a89%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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/CAFwN1uq-BnAX8HyEQDRRnm1wg83%2BEJ4kMVgc_fVTsTu6EU8L5g%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Tim Graham
2016-03-06 00:44:56 UTC
Permalink
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.
Post by 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
Post by 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/
--
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
<javascript:>.
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
<https://groups.google.com/d/msgid/django-developers/2bdf074c-5e21-48cf-a93c-ba19f7744a89%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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/dbc29625-3af4-4b73-9b60-78d269352d52%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Andrew Godwin
2016-03-06 00:49:06 UTC
Permalink
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
Post by 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.
Post by 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
Post by 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/
--
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
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
<https://groups.google.com/d/msgid/django-developers/2bdf074c-5e21-48cf-a93c-ba19f7744a89%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
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/dbc29625-3af4-4b73-9b60-78d269352d52%40googlegroups.com
<https://groups.google.com/d/msgid/django-developers/dbc29625-3af4-4b73-9b60-78d269352d52%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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/CAFwN1urzt3VjQG-LZWz0sevcLGG5S%3DpM4CEMjkf__BytC0%2BsVg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Tim Graham
2016-05-04 00:57:34 UTC
Permalink
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
Post by 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
Post by 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.
Post by 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
Post by 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/
--
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
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
<https://groups.google.com/d/msgid/django-developers/2bdf074c-5e21-48cf-a93c-ba19f7744a89%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
<javascript:>.
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/dbc29625-3af4-4b73-9b60-78d269352d52%40googlegroups.com
<https://groups.google.com/d/msgid/django-developers/dbc29625-3af4-4b73-9b60-78d269352d52%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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/a5d0f33d-882f-4e02-9191-281594ff5522%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Andrew Godwin
2016-05-04 01:03:53 UTC
Permalink
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
Post by 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
Post by 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
Post by 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.
Post by 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
Post by 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/
--
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
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
<https://groups.google.com/d/msgid/django-developers/2bdf074c-5e21-48cf-a93c-ba19f7744a89%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
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/dbc29625-3af4-4b73-9b60-78d269352d52%40googlegroups.com
<https://groups.google.com/d/msgid/django-developers/dbc29625-3af4-4b73-9b60-78d269352d52%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
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/a5d0f33d-882f-4e02-9191-281594ff5522%40googlegroups.com
<https://groups.google.com/d/msgid/django-developers/a5d0f33d-882f-4e02-9191-281594ff5522%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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/CAFwN1up85ycddSNdLvuGWEbHuu2Bnr6tswPYtJdfh0MFS-ppQA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Mark Lavin
2016-05-04 13:15:40 UTC
Permalink
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
Post by 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
Post by 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
Post by 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
Post by 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.
Post by 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
Post by 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/
--
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,
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
<https://groups.google.com/d/msgid/django-developers/2bdf074c-5e21-48cf-a93c-ba19f7744a89%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
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/dbc29625-3af4-4b73-9b60-78d269352d52%40googlegroups.com
<https://groups.google.com/d/msgid/django-developers/dbc29625-3af4-4b73-9b60-78d269352d52%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
<javascript:>.
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/a5d0f33d-882f-4e02-9191-281594ff5522%40googlegroups.com
<https://groups.google.com/d/msgid/django-developers/a5d0f33d-882f-4e02-9191-281594ff5522%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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/0e287bef-c8a7-4a12-a5bc-fed3f05d57c1%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Andrew Godwin
2016-05-04 14:25:53 UTC
Permalink
Post by 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?
- 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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/CAFwN1uqfj7720_qHbO2sggF076w4dCOzaRD%2B7qA%3DwrNG%2BcLhFA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Mark Lavin
2016-05-04 15:26:04 UTC
Permalink
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
Post by Andrew Godwin
Post by 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?
- 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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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.
For more options, visit https://groups.google.com/d/optout.
Andrew Godwin
2016-05-04 16:38:33 UTC
Permalink
Post by Mark Lavin
As noted in the PR there is at least one new setting,
CHANNEL_SESSION_ENGINE, which is lacking documentation.
That's been added now.
Post by Mark Lavin
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.
Post by Mark Lavin
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.
Post by Mark Lavin
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)
Post by Mark Lavin
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.
Post by Mark Lavin
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.
Post by Mark Lavin
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.
Post by Mark Lavin
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.
Post by Mark Lavin
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.
Post by Mark Lavin
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
Post by Mark Lavin
Post by Andrew Godwin
Post by 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?
- 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
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
<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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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.
For more options, visit https://groups.google.com/d/optout.
Markus Holtermann
2016-05-04 17:06:30 UTC
Permalink
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
Post by Andrew Godwin
Post by Mark Lavin
As noted in the PR there is at least one new setting,
CHANNEL_SESSION_ENGINE, which is lacking documentation.
That's been added now.
Post by Mark Lavin
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.
Post by Mark Lavin
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.
Post by Mark Lavin
Channel/Message/Group/ChannelLayerManager/custom JSON encoder classes and
their usage,
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)
Post by Mark Lavin
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.
Post by Mark Lavin
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.
Post by Mark Lavin
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.
Post by Mark Lavin
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.
Post by Mark Lavin
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.
Post by Mark Lavin
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
Post by Mark Lavin
Post by Andrew Godwin
Post by 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?
- 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
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
<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.
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.
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/20160504170630.GF9627%40inel.local.
For more options, visit https://groups.google.com/d/optout.
Andrew Godwin
2016-05-04 17:09:09 UTC
Permalink
Post by 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?
```
from asgiref import ...
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
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/CAFwN1ursVi2g%2BFCj1vY5w%3Df6LOtYyxZackEFhinVF3oBD%2BCPvQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Mark Lavin
2016-05-04 19:24:40 UTC
Permalink
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.
On Wed, May 4, 2016 at 10:06 AM, Markus Holtermann <
Post by 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?
```
from asgiref import ...
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
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/c696555b-4d08-40e6-b1b5-f3e492b60249%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Mark Lavin
2016-05-04 18:52:34 UTC
Permalink
Post by Andrew Godwin
Post by Mark Lavin
As noted in the PR there is at least one new setting,
CHANNEL_SESSION_ENGINE, which is lacking documentation.
That's been added now.
Post by Mark Lavin
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.
Post by Andrew Godwin
Post by Mark Lavin
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.
Post by Mark Lavin
Channel/Message/Group/ChannelLayerManager/custom JSON encoder classes and
their usage,
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.
Post by Andrew Godwin
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.
Post by Andrew Godwin
Post by Mark Lavin
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.
Post by Mark Lavin
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.
Post by Andrew Godwin
Post by Mark Lavin
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.
Post by Andrew Godwin
Post by Mark Lavin
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.
Post by Andrew Godwin
Post by Mark Lavin
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
Post by Mark Lavin
Post by Andrew Godwin
Post by 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?
- 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
<javascript:>.
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
<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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/910b2c17-24fb-450b-acf0-cb027b2aed07%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Andrew Godwin
2016-05-04 19:17:32 UTC
Permalink
Post by Mark Lavin
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.
Post by Andrew Godwin
Post by Mark Lavin
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.
Post by Mark Lavin
Channel/Message/Group/ChannelLayerManager/custom JSON encoder classes
and their usage,
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.
Post by Mark Lavin
Post by Andrew Godwin
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).
Post by Mark Lavin
Post by Andrew Godwin
Post by Mark Lavin
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.
Post by Mark Lavin
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.
Post by Mark Lavin
Post by Andrew Godwin
Post by Mark Lavin
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.
Post by Mark Lavin
Post by Andrew Godwin
Post by Mark Lavin
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
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/CAFwN1up4q6todUDKmxJ_m4voz_%3D6Je-U0bRGrSfNCDkL%2B%3DthBg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Mark Lavin
2016-05-04 19:28:47 UTC
Permalink
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
Post by Andrew Godwin
Post by Mark Lavin
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.
Post by Andrew Godwin
Post by Mark Lavin
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.
Post by Mark Lavin
Channel/Message/Group/ChannelLayerManager/custom JSON encoder classes
and their usage,
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.
Post by Mark Lavin
Post by Andrew Godwin
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).
Post by Mark Lavin
Post by Andrew Godwin
Post by Mark Lavin
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.
Post by Mark Lavin
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.
Post by Mark Lavin
Post by Andrew Godwin
Post by Mark Lavin
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.
Post by Mark Lavin
Post by Andrew Godwin
Post by Mark Lavin
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.
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
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/5bf5537e-0d6f-4803-b391-7824d61d4ac7%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Andrew Godwin
2016-05-04 19:36:26 UTC
Permalink
Post by 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.
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
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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.
For more options, visit https://groups.google.com/d/optout.
Marc Tamlyn
2016-05-04 21:45:01 UTC
Permalink
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
Post by Andrew Godwin
Post by 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.
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
--
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
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
<https://groups.google.com/d/msgid/django-developers/CAFwN1uqpLNTLDuZzNQzH_4k_ZYzHx0pbuXzeKo7u80MjVMr3HA%40mail.gmail.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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/CAMwjO1Fx08P2LOPdgDQoav35Vb-gYXiMyrZXhXO3SvjqgzHPCQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Jacob Kaplan-Moss
2016-05-04 22:47:27 UTC
Permalink
Post by 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.
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
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/CAK8PqJEzFaLo1rbVZ6gxumRrne6NTxx8Qq0Tp77MaB1UKu0-Ew%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Mark Lavin
2016-05-05 00:41:48 UTC
Permalink
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
Post by Jacob Kaplan-Moss
Post by 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.
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
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/218e4513-91b1-4fad-b18c-b6b0e264fdbc%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Tim Graham
2016-05-05 01:12:35 UTC
Permalink
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).
Post by 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
Post by Jacob Kaplan-Moss
Post by 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.
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
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/261d8ffc-3f80-4a05-8e49-0d682aca80e6%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Andrew Godwin
2016-05-05 01:58:17 UTC
Permalink
Post by 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 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
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/CAFwN1upBdqeMgxrXmvuDtFnDSe%2BtD61FMTDhv0GA_iyvnup7gQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Russell Keith-Magee
2016-05-05 02:00:07 UTC
Permalink
Hi Mark,
Post by 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.
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.
Post by Mark Lavin
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 %-)
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/CAJxq8496_ubAO38nszXeb-eknunhwmUXXrrVG8_-RwHS7qHARA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Mark Lavin
2016-05-05 05:53:41 UTC
Permalink
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
Post by Russell Keith-Magee
Hi Mark,
Post by 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.
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.
Post by Mark Lavin
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 %-)
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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.
For more options, visit https://groups.google.com/d/optout.
Aymeric Augustin
2016-05-05 07:29:24 UTC
Permalink
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 <https://myks.org/en/multiple-template-engines-for-django/dep/>, weekly updates <https://myks.org/en/multiple-template-engines-for-django/> — 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.
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
Hi Mark,
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 %-)
--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
Visit this group at https://groups.google.com/group/django-developers <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 <https://groups.google.com/d/msgid/django-developers/455347ad-689c-49c2-a1c0-ace3127a3583%40googlegroups.com?utm_medium=email&utm_source=footer>.
For more options, visit https://groups.google.com/d/optout <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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/2E58214D-CFEB-49E7-9086-237F5848512E%40polytechnique.org.
For more options, visit https://groups.google.com/d/optout.
Anssi Kääriäinen
2016-05-05 07:55:37 UTC
Permalink
Post by Russell Keith-Magee
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
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/CALMtK1FoOSiGr0w_3JP8UYE-_owqOnS59d%2BOjp8eTWt4Qh48SQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Andrew Godwin
2016-05-05 17:07:04 UTC
Permalink
Post by Anssi Kääriäinen
Post by Russell Keith-Magee
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
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.
Post by Anssi Kääriäinen
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.
Post by Anssi Kääriäinen
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.
Post by Anssi Kääriäinen
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).
Post by Anssi Kääriäinen
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
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/CAFwN1uohyt-yTM9NkVg2XCBqtmBP5Xr1a8gZJb%2BET1v-0K72Yg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Anssi Kääriäinen
2016-05-06 04:28:52 UTC
Permalink
Post by Andrew Godwin
Do you have a link to the presentation about them removing it?
around 34 minutes and onwards, another version
is
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
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/CALMtK1FDXaa2kMAo69NO7ji-Cpd4sdTwDWgaysx9NRGH%2BGH%2BSw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Andrew Godwin
2016-05-06 05:15:49 UTC
Permalink
Post by Anssi Kääriäinen
Post by Andrew Godwin
Do you have a link to the presentation about them removing it?
http://youtu.be/839rskyJaro around 34 minutes and onwards, another
version is http://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).
Post by Anssi Kääriäinen
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.
Post by Anssi Kääriäinen
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
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/CAFwN1upU%2B_jm4VHP9C%2B1dtfsrfNqyWHd1Q9yAJ-jvcm5%3DULC%3DQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Tim Graham
2016-05-10 01:42:35 UTC
Permalink
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.
Post by Andrew Godwin
Post by Anssi Kääriäinen
Post by Andrew Godwin
Do you have a link to the presentation about them removing it?
http://youtu.be/839rskyJaro around 34 minutes and onwards, another
version is http://youtu.be/2dG5HeM7MvA at 24 minutes.
- 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).
Post by Anssi Kääriäinen
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.
Post by Anssi Kääriäinen
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
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/ee9ed1e8-4030-4696-ac0f-536d2d4cb097%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Andrew Godwin
2016-05-10 01:54:27 UTC
Permalink
Post by 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.
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.
Post by Tim Graham
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.
Post by Tim Graham
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
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/CAFwN1up7ECX1OyOQHgo9cKb7F2fvvh-Ekzp1WQtmz2NqTp88JA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Anssi Kääriäinen
2016-05-10 06:55:11 UTC
Permalink
Post by Andrew Godwin
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
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/CALMtK1GRgBGwhuEqhBf%3D%3DYyJr%2BF51dVo4CqerGPBNLfwu02hZA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Tom Christie
2016-05-10 08:07:00 UTC
Permalink
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
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/7ab212b4-340d-41f3-a560-61831a9918e5%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Yo-Yo Ma
2016-05-10 14:03:03 UTC
Permalink
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.).
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/bcbbf085-a11c-4894-a599-aa7867899e71%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Yo-Yo Ma
2016-05-10 14:04:10 UTC
Permalink
Correction:

*JKM starred (not started) - stupid, stupid iPhone.
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/cd8c2845-d1bd-479d-8203-a742584d5954%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Tom Christie
2016-05-10 14:53:59 UTC
Permalink
Post by Yo-Yo Ma
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.
Post by Yo-Yo Ma
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. :)
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/df8906f9-d4ea-41b2-b2c5-57e500a1f01d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Asif Saifuddin
2016-03-07 05:55:48 UTC
Permalink
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
Post by 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/
--
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-developers+***@googlegroups.com.
To post to this group, send email to django-***@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/c34f919b-788b-4987-aa80-7085f9aefde4%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Loading...