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

Allow specifying root URLs separately from endpoints #92

Closed
kpreid opened this issue Aug 5, 2017 · 4 comments
Closed

Allow specifying root URLs separately from endpoints #92

kpreid opened this issue Aug 5, 2017 · 4 comments
Assignees

Comments

@kpreid
Copy link
Owner

kpreid commented Aug 5, 2017

Add a configuration option to specify the root URLs explicitly rather than deriving them from the endpoint options. This will enable using ShinySDR behind a reverse proxy, port forwarding, or other such configurations.

Relatedly, it would be nice to have a standard reverse proxy configuration (e.g. for nginx which I vaguely hear is suitable for this task) that makes ShinySDR use only one port rather than two.

@thefinn93
Copy link

I would like to begin working on this, and have a few questions about how you would prefer to have it done.

It looks like the most reasonable place to specify it in the config would be in parameters of config.serve_web, which would then get passed to shinysdr.i.network.app.WebService, but there's a comment there indicating you feel there are already too many parameters. I could create setter methods to configure the websocket URIs there, but that feels rather awkward. What would you suggest? This is just based on me poking around and trying to understand the code, I dont really have a good understanding of the whole codebase, so there may be a better option...

@kpreid
Copy link
Owner Author

kpreid commented Nov 13, 2017

Certainly this should be additional parameters to config.serve_web, as that makes sense from the config-file-design perspective. (I currently think a good name for them would be base_(http|ws)_url; not using the word "root" because it has another use.)

The “too many parameters” note on WebService is more "this is suspicious and should probably be refactored" than any specific desired upper limit. Feel free to just add it there too for now. Don't add setters for this reason; immutable things are to be preferred over mutable things.

Actually, I think it would make sense to have an object (namedtuple or otherwise) that pairs up the endpoint with the URL; then endpoint_string_to_url can also become a method of that object.

@kpreid
Copy link
Owner Author

kpreid commented Apr 19, 2020

As of commit 15ba713, there are two new parameters to config.serve_web(...), http_base_url and ws_base_url, which allow overriding the URL generation. I have not yet tested this with an actual reverse proxy.

@kpreid kpreid closed this as completed Apr 19, 2020
@kpreid
Copy link
Owner Author

kpreid commented Apr 19, 2020

Belated followup: with f78ecfd the ws_base_url is allowed to have a path, which is likely to be useful in reverse proxy configurations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants