Skip to content

Conversation

@martyngigg
Copy link
Contributor

@martyngigg martyngigg commented Sep 3, 2024

SUMMARY

Introduces changes in the front and backend to allow Superset to be deployed under a prefixed-URL path rather than the root of the domain.

The backend superset.app:create_app has been augemented with a new app_root argument that accepts a root path for the application. This is passed through either a SUPERSET_APP_ROOT environment variable or by customizing the FLASK_APP environment variable or the flask --app cli argument, e.g. FLASK_APP="superset:create_app(superset_app_root='/analytics')"

When the path is not / a middleware is created that intercepts paths and updates them to strip the app_root and forward them to the rest of the application. This was inspired by Application Dispatching.

The backend has also been refactored to avoid using hardcoded paths in redirects and instead uses Flask.url_for to construct all references.

The frontend has been updated to include the prefix in any resource links along with two new helper utilities, assetUrl & pathUtils, to help construct the paths. The SupersetClient has been updated to remove the unused baseUrl field and rename it as basePath such that the object can be initialized with the base path and calling any endpoints through this class can use the same relative link as before. QUESTION: Have I broken an API here?

Documentation has been added to the "Configuring Superset" pages that demonstrate how to use the new argument.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@rusackas
Copy link
Member

rusackas commented Sep 5, 2024

Thanks for tackling this... it's a popular request, no doubt about it. If you think it's relevant, we might also want to add an entry in the documentation somewhere.

@martyngigg
Copy link
Contributor Author

Thanks for tackling this... it's a popular request, no doubt about it. If you think it's relevant, we might also want to add an entry in the documentation somewhere.

Yes agreed I definitely need to add some documentation for how to use this. I wanted to wait until I was more confident that the general approach was acceptable before working on describing how to use it. In terms of a review as part of #26319 is there anything I need to do at the moment?

@mistercrunch
Copy link
Member

mistercrunch commented Sep 19, 2024

I think this may be quite an undertaking and might have to be tackled in phases. Would love to see this done :)

I remember giving it a shot early on in the project at least once, and never wrapped up the work. My main advice would be to avoid taking in side mission or bigger refactor while tackling this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's a recent fix in master for this

@ravikantkml
Copy link

Thanks for tackling this... it's a popular request, no doubt about it. If you think it's relevant, we might also want to add an entry in the documentation somewhere.

Yes agreed I definitely need to add some documentation for how to use this. I wanted to wait until I was more confident that the general approach was acceptable before working on describing how to use it. In terms of a review as part of #26319 is there anything I need to do at the moment?

Thanks for these changes.
Can you please mention values for BASE_PATH and ASSET_BASE_URL. Just wanted to check these changes with superset and nginx.

@kasper-rutten
Copy link

For what it's worth, we've been running this fork to serve superset on a path prefix in a Kubernetes(K3S) + traefik context without any problems so far. Though testing has been limited.

Thanks for the work so far

@martyngigg martyngigg force-pushed the allow_prefixed_paths branch from 93d66c7 to 09a05b3 Compare October 15, 2024 14:23
@github-actions github-actions bot added the doc Namespace | Anything related to documentation label Oct 15, 2024
@martyngigg martyngigg force-pushed the allow_prefixed_paths branch from 09a05b3 to fe3b01a Compare October 15, 2024 14:51
@martyngigg
Copy link
Contributor Author

Thanks for tackling this... it's a popular request, no doubt about it. If you think it's relevant, we might also want to add an entry in the documentation somewhere.

Yes agreed I definitely need to add some documentation for how to use this. I wanted to wait until I was more confident that the general approach was acceptable before working on describing how to use it. In terms of a review as part of #26319 is there anything I need to do at the moment?

Thanks for these changes. Can you please mention values for BASE_PATH and ASSET_BASE_URL. Just wanted to check these changes with superset and nginx.

Sorry for the delay in replying to this. I've had zero time to spend on this lately.

The latest commits rebase on the current master and I have also added some documentation to the configuring superset pages that describe the new variables with an example nginx.conf. Does that help?

@martyngigg
Copy link
Contributor Author

@rusackas I think this is ready for review but in terms of it being related to SIP-112 is that the right thing to do in terms of the process?

@rusackas
Copy link
Member

If you think it's ready for review, I'd say you can go ahead and move it from Draft to "Ready for Review" - though it looks like CI might need a little more love still.

As for the SIP, since @sfirke reopened the GitHub issue, I set the SIP itself back to "pre-discussion" state for its official consensus. Did you want to re-kindle that pre-existing SIP, or would it be easier to open a new one? I'm happy to help you carry that SIP through to fruition if you'd like. Feel free to DM me on slack if it's easier.

@ascendlin
Copy link

When clicking on the user list or role list, and then clicking on datasets, a 404 error appears.

@ravikantkml
Copy link

Thanks for tackling this... it's a popular request, no doubt about it. If you think it's relevant, we might also want to add an entry in the documentation somewhere.

Yes agreed I definitely need to add some documentation for how to use this. I wanted to wait until I was more confident that the general approach was acceptable before working on describing how to use it. In terms of a review as part of #26319 is there anything I need to do at the moment?

Thanks for these changes. Can you please mention values for BASE_PATH and ASSET_BASE_URL. Just wanted to check these changes with superset and nginx.

Sorry for the delay in replying to this. I've had zero time to spend on this lately.

The latest commits rebase on the current master and I have also added some documentation to the configuring superset pages that describe the new variables with an example nginx.conf. Does that help?

Thanks a lot for the response.
We tried the changes as recommended and observed that most of the things are working when we specify prefix. However there are few things where prefix is not applied correctly as shown in the image below (chart menu has prefix applied correctly)
image

Also dashboard menu items export to PDF/image not working (Dashboard -> Download -> Export to PDF/Download as Image)

@ravikantkml
Copy link

Thanks for tackling this... it's a popular request, no doubt about it. If you think it's relevant, we might also want to add an entry in the documentation somewhere.

Yes agreed I definitely need to add some documentation for how to use this. I wanted to wait until I was more confident that the general approach was acceptable before working on describing how to use it. In terms of a review as part of #26319 is there anything I need to do at the moment?

Thanks for these changes. Can you please mention values for BASE_PATH and ASSET_BASE_URL. Just wanted to check these changes with superset and nginx.

Sorry for the delay in replying to this. I've had zero time to spend on this lately.
The latest commits rebase on the current master and I have also added some documentation to the configuring superset pages that describe the new variables with an example nginx.conf. Does that help?

Thanks a lot for the response. We tried the changes as recommended and observed that most of the things are working when we specify prefix. However there are few things where prefix is not applied correctly as shown in the image below (chart menu has prefix applied correctly) image

Also dashboard menu items export to PDF/image not working (Dashboard -> Download -> Export to PDF/Download as Image)

I tried to fix above issue of missing prefixes in top-level menu. Please check this PR 2

@Dinesh-4320
Copy link

The solution really works. Solved a long term issue in our deployment behind proxies. Hoping to get merged soon with official code base

@harri2012
Copy link

Very good, looking forward to merging the code soon

@watercraft
Copy link

For whom it may concern, I'm testing with a port of this work to 3.1.1rc1 available from watercraft:superset branch prefix

@jeanpommier
Copy link

Hi,
Good job there, thank you very much for tackling this. I have just discovered that Superset did not support that and was feeling disappointed.

I can see this PR is still flagged as DRAFT. What's left to do to get it "Ready for Review" ?

@martyngigg do you need help in completing this PR ? I'm quite a superset newbie, but I'm willing to provide some help if necessary to get this merged.

@ryuuc
Copy link

ryuuc commented Apr 10, 2025

For those who want to try deploying superset to k8s using the Helm chart before its official release, the following modifications are required:

  1. custom image tag, I found it from github ci https://github.com/apache/superset/actions/runs/14366622555/job/40281019265 , use the py311 version, since wsgiref.types does not exist on Python 3.10, there is a pr to fix https://github.com/apache/superset/pull/33072/files#diff-9689cb75a0ebe47604d59e0a54c904040cd60b45a283a87d0f8d77cac60751d8L21
image:
  repository: apachesuperset.docker.scarf.sh/apache/superset
  tag: 09b92e7-py311
  pullPolicy: IfNotPresent
  1. install the required dependencies. refer to ModuleNotFoundError: No module named 'psycopg2' during k8 installation #31431, but need to use uv instead of pip
bootstrapScript: |
  #!/bin/bash

  # Install system-level dependencies
  apt-get update && apt-get install -y \
    python3-dev \
    default-libmysqlclient-dev \
    build-essential \
    pkg-config 

  # Install required Python packages
  uv pip install --native-tls \
    authlib \
    psycopg2-binary \
    mysqlclient

  # Create bootstrap file if it doesn't exist
  if [ ! -f ~/bootstrap ]; then
    echo "Running Superset with uid {{ .Values.runAsUser }}" > ~/bootstrap
  fi
  1. Add SUPERSET_APP_ROOT env variables, and update health check paths
supersetNode:
  env:
    SUPERSET_APP_ROOT: /your-path
  livenessProbe:
    httpGet:
      path: /your-path/health
  readinessProbe:
    httpGet:
      path: /your-path/health
  startupProbe:
    httpGet:
      path: /your-path/health

landryb added a commit to georchestra/ansible that referenced this pull request Apr 10, 2025
#140)

the branch is rebased on top of upstream's master, ahead of 5.0.0rc2,
and includes:
- apache/superset#30134 (deploy under a prefix)
- apache/superset#33059 (missing prune.py)
- apache/superset#33060 (fix invalid query strings on role list page)

assets have been rebuilt with SCARF_ANALYTICS=false in the env,
thus the tracking pixel is correctly disabled (cf georchestra/superset#1)
return redirect(f"/superset/dashboard/{new_dashboard.id}/?edit=true")
return redirect(
url_for(
"Superset.dashboard", dashboard_id_or_slug=new_dashboard.id, edit=True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow this line broke some tests on master but I don't understand how it didn't get caught by CI (????).

Issues is with the capitalize True here... Maybe you IDE did this? Linter?

@mistercrunch
Copy link
Member

Debugging something funky that happening with this PR, somehow it failed when it merged on master and digging in the last commit's run here https://github.com/apache/superset/actions/runs/14382934496/job/40331066514, it appears that it skipped the python-related test, presumably because of an issue with our logic that checks which files have been touched by the PR and triggers/skips based on whether it sees the backend/frontend getting touched.

I'm guessing the issue is around the logic checking for files touched by the PR, and specific to [maybe] having a merge-based workflow instead of a rebase-type workflow.

Regardless, tests fail here and it's a bit tricky to sort this out, could use some help here to fix master

@mistercrunch
Copy link
Member

Related: #33092

@mistercrunch
Copy link
Member

@martyngigg anyway you can fix the issues that landed through this PR on master? I tried and fix some but didn't have full context. Would prefer not reverting this PR

@martyngigg
Copy link
Contributor Author

@martyngigg anyway you can fix the issues that landed through this PR on master? I tried and fix some but didn't have full context. Would prefer not reverting this PR

Sure. I'll take a look now.

@martyngigg
Copy link
Contributor Author

It looks like the Python unit tests on HEAD of master seem to have a different failure:

/opt/hostedtoolcache/Python/3.10.16/x64/lib/python3.10/site-packages/_pytest/config/__init__.py:331: PluggyTeardownRaisedWarning: A plugin raised an exception during an old-style hookwrapper teardown.
Plugin: helpconfig, Hook: pytest_cmdline_parse
ConftestImportFailure: ModuleNotFoundError: No module named 'wsgiref.types' (from /home/runner/work/superset/superset/tests/conftest.py)
For more information see https://pluggy.readthedocs.io/en/stable/api_reference.html#pluggy.PluggyTeardownRaisedWarning
  config = pluginmanager.hook.pytest_cmdline_parse(
ImportError while loading conftest '/home/runner/work/superset/superset/tests/conftest.py'.
tests/conftest.py:41: in <module>
    from tests.integration_tests.test_app import app
tests/integration_tests/test_app.py:20: in <module>
    from superset.app import create_app
superset/__init__.py:20: in <module>
    from superset.app import create_app  # noqa: F401
superset/app.py:21: in <module>
    from wsgiref.types import StartResponse, WSGIApplication, WSGIEnvironment
E   ModuleNotFoundError: No module named 'wsgiref.types'

Is that expected?

@kgabryje
Copy link
Member

It looks like the Python unit tests on HEAD of master seem to have a different failure:

/opt/hostedtoolcache/Python/3.10.16/x64/lib/python3.10/site-packages/_pytest/config/__init__.py:331: PluggyTeardownRaisedWarning: A plugin raised an exception during an old-style hookwrapper teardown.
Plugin: helpconfig, Hook: pytest_cmdline_parse
ConftestImportFailure: ModuleNotFoundError: No module named 'wsgiref.types' (from /home/runner/work/superset/superset/tests/conftest.py)
For more information see https://pluggy.readthedocs.io/en/stable/api_reference.html#pluggy.PluggyTeardownRaisedWarning
  config = pluginmanager.hook.pytest_cmdline_parse(
ImportError while loading conftest '/home/runner/work/superset/superset/tests/conftest.py'.
tests/conftest.py:41: in <module>
    from tests.integration_tests.test_app import app
tests/integration_tests/test_app.py:20: in <module>
    from superset.app import create_app
superset/__init__.py:20: in <module>
    from superset.app import create_app  # noqa: F401
superset/app.py:21: in <module>
    from wsgiref.types import StartResponse, WSGIApplication, WSGIEnvironment
E   ModuleNotFoundError: No module named 'wsgiref.types'

Is that expected?

I think that the problem is that wsgiref types is available since Python 3.11 and master branch runs on 3.10. But I can’t understand why those tests passed in this branch though…

@martyngigg
Copy link
Contributor Author

@kgabryje Thanks. #33902 contains fixes for the touched files logic that meant that the tests didn't run here. #33095 contains the fixes for those tests that I hadn't realised I'd broken.

@mistercrunch
Copy link
Member

Reporting that in master, simply clicking on "Datasets" in the navbar at the top leads to a 404. Same with SQL Lab, Saved Queries and many others:
Screenshot 2025-04-12 at 2 14 41 PM

@kgabryje
Copy link
Member

Reporting that in master, simply clicking on "Datasets" in the navbar at the top leads to a 404. Same with SQL Lab, Saved Queries and many others: Screenshot 2025-04-12 at 2 14 41 PM

Opened a fix here #33114

@wqbbs911
Copy link

For those who want to try deploying superset to k8s using the Helm chart before its official release, the following modifications are required:

  1. custom image tag, I found it from github ci https://github.com/apache/superset/actions/runs/14366622555/job/40281019265 , use the py311 version, since wsgiref.types does not exist on Python 3.10, there is a pr to fix https://github.com/apache/superset/pull/33072/files#diff-9689cb75a0ebe47604d59e0a54c904040cd60b45a283a87d0f8d77cac60751d8L21
image:
  repository: apachesuperset.docker.scarf.sh/apache/superset
  tag: 09b92e7-py311
  pullPolicy: IfNotPresent
  1. install the required dependencies. refer to ModuleNotFoundError: No module named 'psycopg2' during k8 installation #31431, but need to use uv instead of pip
bootstrapScript: |
  #!/bin/bash

  # Install system-level dependencies
  apt-get update && apt-get install -y \
    python3-dev \
    default-libmysqlclient-dev \
    build-essential \
    pkg-config 

  # Install required Python packages
  uv pip install --native-tls \
    authlib \
    psycopg2-binary \
    mysqlclient

  # Create bootstrap file if it doesn't exist
  if [ ! -f ~/bootstrap ]; then
    echo "Running Superset with uid {{ .Values.runAsUser }}" > ~/bootstrap
  fi
  1. Add SUPERSET_APP_ROOT env variables, and update health check paths
supersetNode:
  env:
    SUPERSET_APP_ROOT: /your-path
  livenessProbe:
    httpGet:
      path: /your-path/health
  readinessProbe:
    httpGet:
      path: /your-path/health
  startupProbe:
    httpGet:
      path: /your-path/health

seems it doesn't work in 5.0.0rc2

alexandrusoare pushed a commit to alexandrusoare/superset that referenced this pull request Jun 19, 2025
@wqbbs911
Copy link

5.0.0 was released, seems still doesn't work

@mistercrunch
Copy link
Member

5.0 was cut in Feb ... took a while to stabilize and release the branch. 6.0 should be a fast-follow though, hoping around a month from now.

@landryb
Copy link
Contributor

landryb commented Jul 4, 2025

this has been merged a while ago and is working fine, but i think the swagger link doesnt have the prefix, eg going to https://instance/path/swagger/v1 tries to fetch /api/v1/_openapi without the prefix (which should be coming from application_root afaict), which yields a 404.

looking at the code but so far i fail to see where this should be configured/handled, eg in FAB or superset frontend itself or somewhere else...

edit bah i guess that's already been reported as #33304

@mistercrunch
Copy link
Member

labeled #33304 as "good first issue" for hopefully someone to pick up.

@Ayoub-BOUCHACHIA
Copy link

I'm using Superset 4.1.2, but the solution isn't working for me. Can anyone explain why?

Here’s the relevant output from my logs:

[notice] A new release of pip is available: 25.0.1 -> 25.2
[notice] To update, run: pip install --upgrade pip
Starting web app (using development server)...
Usage: flask run [OPTIONS]
Try 'flask run --help' for help.

Error: The factory "create_app(superset_app_root='/archipel')" in module 'superset' could not be called with the specified arguments.
+ '[' 0 -eq 1 ']'
+ [[ 1 -gt 0 ]]
+ [[ dev == \d\e\v ]]
+ echo 'Skipping gunicorn, running app in dev mode'
+ bash /app/docker/docker-bootstrap.sh app
Skipping gunicorn, running app in dev mode
Installing local overrides at /app/docker/requirements-local.txt
Requirement already satisfied: psycopg2-binary==2.9.6 in /usr/local/lib/python3.10/site-packages (from -r /app/docker/requirements-local.txt (line 1)) (2.9.6)
WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager, possibly rendering your system unusable. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv. Use the --root-user-action option if you know what you are doing and want to suppress this warning.

[notice] A new release of pip is available: 25.0.1 -> 25.2
[notice] To update, run: pip install --upgrade pip
Starting web app (using development server)...
Usage: flask run [OPTIONS]
Try 'flask run --help' for help.

Error: The factory "create_app(superset_app_root='/archipel')" in module 'superset' could not be called with the specified arguments.

@jeanpommier
Copy link

Hi @Ayoub-BOUCHACHIA
I don't think it's been ported to 4.1.2.
Actually, AFAIK, it's not part of any release at the current time: it's been merged on master branch, but not released yet.

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

Labels

api Related to the REST API change:backend Requires changing the backend change:frontend Requires changing the frontend doc Namespace | Anything related to documentation github_actions Pull requests that update GitHub Actions code packages size/XXL

Projects

None yet

Development

Successfully merging this pull request may close these issues.