Skip to content
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

Make WebSocket URL relative to path on which RustPad itself is served #78

Merged
merged 2 commits into from
Sep 4, 2024

Conversation

ntninja
Copy link
Contributor

@ntninja ntninja commented Jul 19, 2024

The path stays the same in all currently supported scenarios, but this also allows serving the app from a subpath.

@ekzhang
Copy link
Owner

ekzhang commented Jul 20, 2024

Hm, can you give an example of where it would be served on a subpath? Is this something supported by reverse proxies / did it come up for you?

@ntninja
Copy link
Contributor Author

ntninja commented Jul 20, 2024

Yes, all reverse proxies I’ve ever used allow adding rules along the lines of forwarding /rustpad/api/* to http://localhost:3000, while serving /rustpad/* from the local file system. It is possible to serve /rustpad/* from the local file system and forwarding /api/* instead, but it clutters the top-level namespace, that in this case would be used by Jitsi Meet with Rustpad being “only” its EtherPad-standin helper service. (I haven’t set this up yet though, since getting the web component to build on NixOS is hard – there is no established way to deal with wasm-pack in that system unfortunately.)

@ntninja
Copy link
Contributor Author

ntninja commented Jul 20, 2024

I needed one more patch to make Vite’s build output use relative links, but not everything (all patches) appear to work.

See final version here if you’re curious:
https://talk.erin-nachhilfe.schlarb.eu/rustpad/#sD2loh?showSidebar=false&userName=SomeUser

@ekzhang
Copy link
Owner

ekzhang commented Jul 20, 2024

Nice, great stuff. Is there a reason you wouldn't just forward /rustpad/:path to localhost:3000/:path?

@ntninja ntninja force-pushed the feat-relative-ws-url branch from 7dce34b to b534201 Compare July 23, 2024 15:30
@ntninja
Copy link
Contributor Author

ntninja commented Jul 23, 2024

@ekzhang: I’m not following unfortunately, why would I forward serving the web client package to the API server?

@ekzhang
Copy link
Owner

ekzhang commented Jul 23, 2024

There are not two separate servers, intended deployment method is just one server for API routes and static files.

That's how the Dockerfile works!

ntninja added 2 commits August 1, 2024 18:37
The path stays the same in all currently supported scenarios, but this also
allows serving the app from a subpath.
@ntninja ntninja force-pushed the feat-relative-ws-url branch from b534201 to 4854ae3 Compare August 1, 2024 16:37
@ntninja
Copy link
Contributor Author

ntninja commented Aug 1, 2024

Ok, I never looked at either the Dockerfile nor the rustpad-server code. If I understand the server code correctly it also serves up static files from a local directory named dist/?

I just assumed I need to server static files separately since that is how all server software I know (which is mostly Python) works. In my current deployment scenario the frontend is also built independently from the server component (I managed to build the web and server components in a smart way now, but they end up in two unrelated packages/derivations only sharing the Cargo dependencies), so it doesn’t make much sense to combine the two if one can just define 2 routes in the web server that is already required for TLS termination…

More importantly, this is all besides the point anyways since I could have just written this instead and it would still name the same issue that this patch solves:

All the reverse proxies I’ve ever used allow adding rules along the lines of forwarding /rustpad/* to http://localhost:3000 (stripping the /rustpad prefix before passing the URL to the backend), thereby allowing a different service to be hosted at /.

Whether static files are served by the web server or the rustpad server isn’t relevant to this issue at all.

@ekzhang
Copy link
Owner

ekzhang commented Sep 4, 2024

server static files separately since that is how all server software I know (which is mostly Python) works

This isn't how the Jupyter server works, for instance. No worries though

@ekzhang
Copy link
Owner

ekzhang commented Sep 4, 2024

I think this makes sense, thank you!

@ekzhang ekzhang merged commit 9b9d369 into ekzhang:main Sep 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants