django / channels

Developer-friendly asynchrony for Django
https://channels.readthedocs.io
BSD 3-Clause "New" or "Revised" License
6.09k stars 800 forks source link

Multiplexer send hijacking #729

Closed claytondaley closed 7 years ago

claytondaley commented 7 years ago

Currently, the multiplexer hijacks the consumer's send method and replaces it with its own exception-raising method. This seems to create two issues:

Is there an issue with the lifecycle of consumers that we cannot either:

andrewgodwin commented 7 years ago

This is being entirely redone with the Channels 2 design, where the send method is an integral part of the consumer abstraction and so multiplexing or encapsulation is easily done.

You can see the in-process work on the "2.0" branch of this repo.

claytondaley commented 7 years ago

Thanks. The new approach seems to address both my concerns. I see that the branch is relatively new. Is there a documented set of goals/changes for the new version?

For example, enforcing permissions is next on my list. I can use DRY decorators for some of the heavy lifting, but would prefer to future-proof my code as much as possible if, for example, you were going to add a dedicated permissions interface that reflected best practices.

andrewgodwin commented 7 years ago

The blog post I wrote is the closest thing to goals (http://www.aeracode.org/2017/07/11/towards-channels-20/) - but it's very much developing as we go. Speed has slowed down recently due to personal illness, but hopefully will get back again soon.

claytondaley commented 7 years ago

Hope your health improves! A picture of old and new would be extremely helpful in putting all of the verbiage together. I also have some reactions, but am not sure the best place to provide them. Perhaps you'd consider creating Git Issues for the various major design changes. That would help persist any Q&A that clarifies the change and facilitate/document conversation.

It'd be really helpful if each of these issues included a more detailed explanation of the problem needing solved. For example, I don't understand your issue with sessions. I'm under the impression that they exist for almost exactly this use case. Maybe your concern is more subtle... like you expect sessions to be used to persist data across rather than within a single server interaction. Is there a performance impact? Or is the concern purely aesthetic?

I happen to love that workers come "for free" with the framework. I'd hate to lose that feature to a fat-protocol without better understanding the session data issue. Can we wrap frames in an envelope instead? The envelope could store within-interaction data without involving the session.

andrewgodwin commented 7 years ago

I'll have more clear descriptions once I have finished working out what I think via the medium of prototyping. As for workers, they're not going away, just becoming more of a separate first-class concept.

claytondaley commented 7 years ago

You don't need to prototype to clearly document the perceived problems and your initial thoughts. That would be enough to invite clarification/ideas before you created a ton of technical debt by prototyping.

It would also invite discussion about perceived problems and their priority. For example, Consumers and Views fulfill similar roles. It would be a lot easier to adopt Channels (and likely DRY) if they had more in common. Instead of making the interfaces more-similar (e.g. mapping multiplexed stream names to methods), your blog proposes to make consumers a callable returning a callable.

Managing authentication and permissions is also a huge learning curve and the source of inordinate amounts of discussion. That seems totally backwards to me. Authentication in a framework should be easy. Permissions should be easy. They're not. Django REST Framework may have spoiled me, but they have a clean, hierarchical permissions system. When I had to tweak error codes, I learned to hate the implementation, but the approach is perfect for most users (and especially new users).

andrewgodwin commented 7 years ago

I don't prototype to find the problems, I prototype to find workable solutions. My goal is to make consumers an encapsulation that's reuseable, supports a middleware-alike, makes authentication and sessions easy, and more.

Yes, I could spend a long time writing up my detailed thoughts around them, but in the past that's had zero reaction or feedback from anyone, while working code generally spurs on more, so that's how I choose to do it.

If you've seen any of the many blog posts or talks I've given over the last couple of years, you'll know that unifying consumers and views is high on my list, but this means making views into consumers, not vice-versa. My goal overall is to try and underpin existing Django with a compatible lower layer, and then build on top of that, not to do two systems side by side.

If my preferred way of working out problems is hard to read - sorry, but there's not exactly a glut of people working on this problem I can collaborate with. I try my best to be transparent where it doesn't take days of typing and documentation, and to talk to people in person when I see them, but it's not like I have a whole team of experts on standby to consult on implementation ideas with. Most of the ideas or outlines I put out are met with zero response, so it's not really worth investing in doing more for me personally at the moment.

claytondaley commented 7 years ago

I suggested almost exactly the refactor you implemented. I suggested a specific (i.e. view-consumer) change that you apparently have been blogging about for a long time (no I don't read your blog). Even if you don't like the approach, I assume you agree that a message envelope is a valid strategy to avoid storing data in sessions. Every bit of evidence should suggest that I (1) recognize the opportunities to improve the platform and (2) come up with valid solutions to those problems (since most are the same as yours!).

So a competent person is interested in this problem, but needs clarification. He suggests having the exchange in public (i.e. issues) in part so future contributors can avoid repeating the discussion.

How would you characterize your response to that potential contributor... and how do you expect them to respond? Do you think he's going to become one of the "people working on this problem [you] can collaborate with"?

andrewgodwin commented 7 years ago

Generally when people accuse me of "creating technical debt by prototyping" and "not documenting the problems", when neither are true of what I'm trying to do with my limited time, then I'm not going to react well to a continued discussion or want to work with that person - I would hope it's just intent mis-percieved via the frame of the internet.

If someone wants to genuinely help out, I'd expect them to be willing to go through and understand previous blog posts, current and previous code, and maybe a talk or two to get some idea of the context and the challenges here. Anyway, I will try and re-explain some of that context for this specific request

The proposed idea of consumers - a callable returning a callable - maps fine to views, as a view is just the inside of those two callables; a decorator that wraps it in another callable that accepts http-type connections and dispatches to the view on http.request events, and takes the view's response and sends it back to the socket, would be plenty.

However, Channels has to work into Django in an entirely backwards-compatible way, which means there can't be a requirement to decorate all your views suddenly when you move over (or do anything else to them). On top of that, the URL routing system is HTTP-specific, so instead the idea is to encapsulate the entirely of the current Django URL and routing as a single consumer (which is also the current approach in channels).

That leaves the approach of how to make consumers seem familiar enough to users coming from normal Django views. We could try and cram it into a single function-style consumer and make people select based on the message.type, much like you would select based on request.method, but instead I prefer to go class-based and mirror the way class-based views work, with methods per message type/method.

As for storing data in sessions, I'm not entirely clear how you propose to store per-socket session data in a message envelope, considering sessions can be written to from multiple sources at once and if they were enveloped they'd have to be stored either at the socket-terminating server or on the client itself. Moving everything to run inside the socket-terminating server itself, as I'm proposing, allows those things (along with authentication, etc) to be stored as class instance variables instead, which is a lot, lot more efficient and more importantly, easier for developers to understand and not screw up.

If you've got a proposal on how to change these, I'd love to see it but it needs to be far enough along that it has implementation details, as most of the difficulty with a project like channels is not the theoretical designing but the practical implementation and doing that in a way where things will be both backwards-compatible, not introduce a lot of tech debt, and still be understandable and safe for new developers. If those weren't concerns, I'd just dump a direct message bus interface on people and let them have at it.

If you're concerned about getting authentication information or user session information (as opposed to the old per-channel sessions) in workers, that's a separate problem that I'm currently thinking about solving with token-based authentication riding along with messages, which may be close to the envelope idea you mentioned. At that point, though, you're starting to invent a light services framework, so I'd like to make sure whatever it is ends up being something that can be easily built upon rather than an overly simplistic dead-end.