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

Fix APPLICATION_ROOT example in docs #1270

Merged
merged 2 commits into from
Nov 23, 2023
Merged

Fix APPLICATION_ROOT example in docs #1270

merged 2 commits into from
Nov 23, 2023

Conversation

aristocrates
Copy link
Contributor

If the application is hosted on the path /somestring, static asset paths need to have prefix /somestring/static/ but setting APPLICATION_ROOT to "somestring" will result in a relative path which will give a 4XX. Using "/somestring" fixes this.

e.g. static assets should look like this in source:

<link rel="stylesheet" type="text/css" href="/somestring/static/css/main.css">

but without the leading slash they have href="somestring/static/css/main.css" which results in a network request for {DOMAIN}/somestring/{PROJECT_NAME}/somestring/static/css/main.css

The test suite includes the leading slash in test_prefixed:

def test_prefixed(self):
self.app.config["APPLICATION_ROOT"] = "/foo"


Also fix default value, which is "/" and not the empty string:

APPLICATION_ROOT = "/"

APPLICATION_ROOT="/" \

- APPLICATION_ROOT=/

# Set this to a URL path under which the application will be served. Defaults to "/"
APPLICATION_ROOT = "/"

self.app.config["APPLICATION_ROOT"] = "/"

If the application is hosted on the path /somestring, static asset
paths need to have prefix /somestring/static/ but setting
APPLICATION_ROOT to "somestring" will result in a relative path which
will give a 4XX. Using "/somestring" fixes this.

Also fix default value, which is "/" and not the empty string.
docs/configuration.md Outdated Show resolved Hide resolved
@almet
Copy link
Member

almet commented Nov 22, 2023

Thanks, that looks good to me.

Default APPLICATION_ROOT is "/", so "By default" is more accurate than
"If empty" for the default value.
@aristocrates
Copy link
Contributor Author

Thanks, I also added the s/If empty/By default/ change

@zorun
Copy link
Collaborator

zorun commented Nov 23, 2023

Thanks for catching the doc issue.

From what I remember from last time I looked at this, the default value from Flask was an empty string. We merely force this to be / in all our example configs, see 3788b6f . But it seems I remember wrong: https://flask.palletsprojects.com/en/2.3.x/config/#APPLICATION_ROOT

By the way, this is all a hack, normally we are supposed to get this variable from the web server / runtime environment, and not define it ourselves. But anyway :)

In any case, this looks good to me!

@zorun zorun added the doc label Nov 23, 2023
@zorun zorun merged commit bb30813 into spiral-project:master Nov 23, 2023
@aristocrates aristocrates deleted the aristocrates/fix_application_root_example branch November 24, 2023 05:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants