-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Add new websockets doc page #2608
Conversation
* Simple explanation. * Example of usage with JS. * Additional nGinx config example.
Edit: I misinterpreted this entirely... I'm stuck in thinking about AWS ELB. LGTM. Thanks for the comment @stefanitsky! Regarding this PR, is it really necessary to set whatever you had in the nginx configuration? (Never tried, but I'm pushing to AWS ALB soon, so this perked my interest since I have to use nginx as a reverse proxy). The headers should just transfer over though, without explicitly specifying, right, for a reverse proxy? Because what I'm sensing in this doc is that the dev set the Unless I'm totally getting this wrong. |
@Andrew-Chen-Wang yes, it is necessary, personally verified on a real project. I removed the traefik image, because im using backend and frontend separately. Config example:
You can verify that by using JS example:
|
Ah I see, I'm misinterpreting this use-case. I'm stuck in a different mindset than before... My mind was in AWS mode for way too long. Just to clarify, this is for self-hosting, right? If so, then thanks for the addition! |
@Andrew-Chen-Wang hehe, and i'm not familiar with AWS, so i don’t know how everything is there. Yes, it's for self-hosting like VDS or something else. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the addition, left a couple of small suggestions.
docs/websocket.rst
Outdated
undefined | ||
pong! | ||
|
||
If you are using nGinx instead of Traefik, you should add these lines to nGinx config: :: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are our writing it nGinx
? Doesn't look like a thing according to Wikipedia. I would stick to Nginx
, unless you can quote a source supporting your spelling.
docs/websocket.rst
Outdated
location /websocket/ { | ||
proxy_pass http://backend; | ||
proxy_http_version 1.1; | ||
proxy_set_header Upgrade $http_upgrade; | ||
proxy_set_header Connection "upgrade"; | ||
proxy_read_timeout 86400; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about adding this to our docs... I appreciate that Nginx is more widely used, but this is a piece of code that I'd rather NOT maintain and instead link to Nginx, which is more likely to remain updated.
Can we rephrase this to something more generic? Something along the lines of
"if you don't use Traefik, you might have to configure your reverse proxy accordingly (example with Nginx)"
docs/websocket.rst
Outdated
Websocket | ||
========= | ||
|
||
You can enable web sockets if you select ``use_async`` option when creating a project. That indicates whether the project should use web sockets with Uvicorn + Gunicorn. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can enable web sockets if you select ``use_async`` option when creating a project. That indicates whether the project should use web sockets with Uvicorn + Gunicorn. | |
You can enable web sockets if you select ``use_async`` option when creating a project. That indicates whether the project can use web sockets with Uvicorn + Gunicorn. |
@browniebroke i completely agree, updated |
Description
Added small docs for websocket that was added in #2506