-
Notifications
You must be signed in to change notification settings - Fork 268
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Daphne-Root-Path not taken in consideration in consumers URLs #125
Comments
Yes, this is a bug. There's likely also a "bug" in the Channels routing code, in that it does not respect the root_path value but instead patches on the raw |
As fast workaround, we could add Also, another consequence for Channels is that urls reverse should not work in websocket consumers, as Fix this (for Daphne) should be easy, we only have to add |
I wanted to try and avoiding overloading the The other option is to improve Channels' routing to actually understand URLs properly and deal with path prefixing (this will also let us fix the inconsistency with leading slashes vs. Django URLs that currently exists). It's a bit of a bigger piece of work, but likely a better overall result. |
Just to make my issue is related (please correct me if I'm wrong). On localhost with runserver, I can reach the websocket connect function in my consumer with the following:
On my production environment, my nginx conf for the websocket proxy looks like this:
I then try to connect to
If my issue is valid and related to this, what would be a suitable workaround ? I could easily create different routes to be used in development and production if needed. |
Yes, that looks like the same issue. For now the workaround is as you showed with the URL config; you could move the prefix part into a setting, maybe, and then use that in the URL config, but really I just need to find some time to fix this. |
Any updates? |
So, it looks like all we need to do to resolve this is have daphne handle the I have not dug into channels prior to 4.0 on this particular topic. In I have tested such changes locally with roost-ng, dropping the middleware I wrote to work around this issue in the past, and it seems to work. @andrewgodwin, is there anything obvious missing here? If not, I'll open PRs with daphne and channels. |
@asedeno If you open PRS with test I'm targeting new releases for the new year period, so if you break ground I'm happy to consider this for those. Thanks. |
@carltongibson the implementation as mentioned is in the PRs that are referencing this bug. PTAL when you have a chance. |
@carltongibson: friendly ping. |
@asedeno "new year period" -- please don't do "friendly pings" it's just noise on my timeline. Thanks. |
@carltongibson I interpreted "targeting new releases for the new year period" as needing to get things in before then, and expect that to take some time, feedback, and iteration. As the sum total of the changes so far are +31-2 for daphne, and +49-0 for channels, mostly in tests, was hoping for feedback sooner rather than later. If you don't actually plan to look at this for weeks, then please set that as the expectation. In the mean time, it would at least be beneficial to unblock workflows for the PRs, so they get tested now and moving forward if I decide to make further changes. Thanks. |
@carltongibson, so the entire new year has passed and we're in a new "new year period" so I figure it's finally time to circle back and see if we can get an update on this. |
Hi @asedeno — Yes, thanks for your patience. (2023 was something of a year TBH.) Let me review this and we'll see where we're at. |
Fixed (on the Daphne side) in #453. django/channels#1954 Provides the Channels part to go with it. |
Daphne does not seem to use the
Daphne-Root-Path
when it computes the urls of the consumers.I have:
Daphne-Root-Path
set toappname
r"^home"
in myurls.py
r"/ws/something"
in myrouting.py
And I observe that:
/appname/home
which means that Daphne is using the root-path correctly./ws/something
and not at/appname/ws/something
which I would expect.Indeed, if I look at the source, I can see that
http_protocol.WebRequest
has aroot_path
attribute which is used when a http answer is sent (here https://github.com/django/daphne/blob/1.3.0/daphne/http_protocol.py#L162) but there is nothing like this inws_protocol.py
.Is this actually a bug or am I missing something?
I tested this with Daphne 1.2.0 but it seems that the issue remains on the lastest version on the repository.
The text was updated successfully, but these errors were encountered: