bxsx commented on code in PR #42495: URL: https://github.com/apache/superset/pull/42495#discussion_r3670073687
########## docker/pythonpath_dev/superset_config.py: ########## @@ -114,7 +114,11 @@ class CeleryConfig: } EXTENSIONS_PATH = "/app/docker/extensions" ALERT_REPORTS_NOTIFICATION_DRY_RUN = True -WEBDRIVER_BASEURL = f"http://superset_app{os.environ.get('SUPERSET_APP_ROOT', '/')}/" # When using docker compose baseurl should be http://superset_nginx{ENV{BASEPATH}}/ # noqa: E501 +# The Docker Compose app service is named "superset" and listens on 8088. Only the +# scheme/host/port matter here (paths are joined with urljoin). For screenshots in +# the dev stack (unbuilt static assets) point this at the nginx service instead: +# http://nginx{SUPERSET_APP_ROOT}/ +WEBDRIVER_BASEURL = f"http://superset:8088{os.environ.get('SUPERSET_APP_ROOT', '/')}/" Review Comment: With the default https://github.com/apache/superset/blob/00c60bf2def2db276cca0c0e614a3cd011e69bde/docker/.env#L71 this does render http://superset:8088// - but that shape is pre-existing on master (this PR only swaps the hostname), and it's inert in every consumer: the report/screenshot machinery joins root-relative paths with `urljoin`, which drops the base path altogether, and the string-concatenating consumers rstrip('/') before appending. No constructed URL ever contains the `//`. Normalizing this one line would be cosmetic and would leave `WEBDRIVER_BASEURL_USER_FRIENDLY` two lines down (same pattern, renders a triple slash) inconsistent - if there's appetite for normalizing `SUPERSET_APP_ROOT` handling, that's better done for both values in a dedicated cleanup than folded into a hostname fix. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
