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

BASEPATH cannot be set via config! #10

Open
svilupp opened this issue May 10, 2023 · 3 comments
Open

BASEPATH cannot be set via config! #10

svilupp opened this issue May 10, 2023 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@svilupp
Copy link

svilupp commented May 10, 2023

First of all, thank you for the awesome productivity hack with @genietools.

I just wanted to point out one unexpected behavior around setting a BASEPATH.
(BASEPATH can be used if you want your URLs to be "127.0.0.1:8000/my/base/path/something" instead of "127.0.0.1:8000/something" for your asset paths. Useful when you use nginx proxy)

Expected behavior: As a user, I would like to set BASEPATH either with ENV["BASEPATH"]="/my/base/path" or with `Genie.Configuration.config!(;base_path="/my/base/path")

Current behavior: Only setting via environment variable BASEPATH works (eg, ENV["BASEPATH"] or passing it when starting Julia BASEPATH="/my/base/path/" julia)

Reason: The following [line] reads BASEPATH directly from ENV without considering the config

Side note: If you want to change your BASEPATH, remember that adjust your routes accordingly (eg, route("/my/base/path/routeA") do .... end) or setup your nginx to remove the extra basepath segment from the forwarded requests!

Discovered in GenieFramework/Stipple.jl#199

Happy to open the PR if there is interest (adding the check for Genie.config.base_path)

@essenciary
Copy link
Member

@svilupp Great point! Did we end up discussing about this in Genie? Because yes, a PR in Genie would be great, thanks :)

@hhaensel
Copy link
Member

hhaensel commented Oct 1, 2023

I think there are more options that are not processed currently, e.g. PORT, WSPORT, WSBASEPATH. MAybe we can look over this and cover them all in one PR?

@essenciary
Copy link
Member

Yes, I'll add them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants