codeant-ai-for-open-source[bot] commented on code in PR #42495: URL: https://github.com/apache/superset/pull/42495#discussion_r3670116372
########## docker/pythonpath_dev/superset_config.py: ########## @@ -114,7 +114,13 @@ 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. Report +# paths are root-relative, so urljoin drops the base path; only the scheme, host, +# and port must be correct here. SUPERSET_APP_ROOT is kept for consumers that +# concatenate paths directly (e.g. cache warm-up). 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: **Suggestion:** The default report URL bypasses the `nginx` service, which is the only service in the dev Compose stack that proxies `/static` assets to the webpack dev server. The `superset` backend does not serve the unbuilt development bundle, so screenshot jobs using this default can load the page without its JavaScript and fail or produce blank screenshots. Use the asset-serving `nginx` endpoint for the dev screenshot path, or otherwise ensure the default URL serves both backend routes and development assets. [api mismatch] <details> <summary><b>Severity Level:</b> Critical ๐จ</summary> ```mdx - โ Development screenshot reports can fail or render blank. - โ Dashboard report delivery uses the affected browser path. - โ ๏ธ Direct backend access bypasses Webpack asset proxying. ``` </details> <details> <summary><b>Steps of Reproduction โ </b></summary> ```mdx 1. Start the development stack with `docker compose up`; `docker-compose.yml:117-123` defines the application service as `superset` and the Webpack development server as `superset-node`. 2. Create a screenshot-based scheduled report, which reaches `AsyncExecuteReportScheduleCommand._get_screenshots()` in `superset/commands/report/execute.py:549-598` and opens the generated dashboard URL in a browser. 3. The generated URL uses `WEBDRIVER_BASEURL` through `headless_url()` in `superset/utils/urls.py:24-31`, so the browser navigates directly to `http://superset:8088`. 4. The development Nginx configuration routes `${SUPERSET_APP_ROOT}/static` to the Webpack server at `docker/nginx/templates/superset.conf.template:40-44`; the direct `superset:8088` endpoint bypasses that route. The page can therefore load without the development JavaScript bundle, causing screenshot rendering to fail or produce a blank/partial image. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4e4451d6ed854d4b8e6eb4e5c5a88b17&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=4e4451d6ed854d4b8e6eb4e5c5a88b17&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) *(Use Cmd/Ctrl + Click for best experience)* <details> <summary><b>Prompt for AI Agent ๐ค </b></summary> ```mdx This is a comment left during a code review. **Path:** docker/pythonpath_dev/superset_config.py **Line:** 123:123 **Comment:** *Api Mismatch: The default report URL bypasses the `nginx` service, which is the only service in the dev Compose stack that proxies `/static` assets to the webpack dev server. The `superset` backend does not serve the unbuilt development bundle, so screenshot jobs using this default can load the page without its JavaScript and fail or produce blank screenshots. Use the asset-serving `nginx` endpoint for the dev screenshot path, or otherwise ensure the default URL serves both backend routes and development assets. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42495&comment_hash=8a32fa4ed46b5327647649f5a80c9a763cd49cf2520ba72febfc50bbef7b3070&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42495&comment_hash=8a32fa4ed46b5327647649f5a80c9a763cd49cf2520ba72febfc50bbef7b3070&reaction=dislike'>๐</a> ########## docs/admin_docs/configuration/alerts-reports.mdx: ########## @@ -402,9 +402,9 @@ Possible fixes: The worker may be unable to reach the report. It will use the value of `WEBDRIVER_BASEURL` to browse to the report. If that route is invalid, or presents an authentication challenge that the worker can't pass, the report screenshot will fail. -Check this by attempting to `curl` the URL of a report that you see in the error logs of your worker. For instance, from the worker environment, run `curl http://superset_app:8088/superset/dashboard/1/`. You may get different responses depending on whether the dashboard exists - for example, you may need to change the `1` in that URL. If there's a URL in your logs from a failed report screenshot, that's a good place to start. The goal is to determine a valid value for `WEBDRIVER_BASEURL` and determine if an issue like HTTPS or authentication is redirecting your worker. +Check this by attempting to `curl` the URL of a report that you see in the error logs of your worker. For instance, from the worker environment, run `curl http://superset:8088/superset/dashboard/1/`. You may get different responses depending on whether the dashboard exists - for example, you may need to change the `1` in that URL. If there's a URL in your logs from a failed report screenshot, that's a good place to start. The goal is to determine a valid value for `WEBDRIVER_BASEURL` and determine if an issue like HTTPS or authentication is redirecting your worker. Review Comment: **Suggestion:** The troubleshooting command uses the legacy `/superset/dashboard/1/` route, which the application redirects to `/dashboard/1/`. Because the example does not use `curl -L`, users may only observe the redirect response and incorrectly diagnose the worker URL as invalid. Use the canonical `/dashboard/1/` path or explicitly follow redirects. [api mismatch] <details> <summary><b>Severity Level:</b> Minor ๐งน</summary> ```mdx - โ ๏ธ Worker URL troubleshooting shows redirect responses. - โ ๏ธ Users may misdiagnose valid connectivity as failure. - โ ๏ธ Report screenshot diagnostics become less clear. ``` </details> <details> <summary><b>Steps of Reproduction โ </b></summary> ```mdx 1. Follow the troubleshooting instructions at `docs/admin_docs/configuration/alerts-reports.mdx:405` from the worker container and run `curl http://superset:8088/superset/dashboard/1/` without `-L`. 2. The application exposes the canonical dashboard handler at `superset/views/core.py:794-796` under `/dashboard/<dashboard_id_or_slug>/`, not the legacy `/superset/dashboard/` path. 3. `LegacyPrefixRedirectMiddleware` documents and implements the legacy conversion from `/superset/<canonical>` to `/<canonical>` in `superset/middleware/legacy_prefix_redirect.py:166-168`; the curl command therefore receives a redirect response rather than the dashboard response. 4. Because curl does not follow redirects by default, users troubleshooting connectivity may inspect only the redirect status/body instead of the actual dashboard response. The example should use `/dashboard/1/` or add `-L`. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ad07f97a91244e5fb8aee3686ccf3977&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=ad07f97a91244e5fb8aee3686ccf3977&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) *(Use Cmd/Ctrl + Click for best experience)* <details> <summary><b>Prompt for AI Agent ๐ค </b></summary> ```mdx This is a comment left during a code review. **Path:** docs/admin_docs/configuration/alerts-reports.mdx **Line:** 405:405 **Comment:** *Api Mismatch: The troubleshooting command uses the legacy `/superset/dashboard/1/` route, which the application redirects to `/dashboard/1/`. Because the example does not use `curl -L`, users may only observe the redirect response and incorrectly diagnose the worker URL as invalid. Use the canonical `/dashboard/1/` path or explicitly follow redirects. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42495&comment_hash=2d0c277521714c7987cc99e0c91ef0c0ce8bc9426b0ff3a1f1437f4f1e775964&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42495&comment_hash=2d0c277521714c7987cc99e0c91ef0c0ce8bc9426b0ff3a1f1437f4f1e775964&reaction=dislike'>๐</a> -- 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]
