Copilot commented on code in PR #40234:
URL: https://github.com/apache/superset/pull/40234#discussion_r3263648580
##########
.github/workflows/bashlib.sh:
##########
@@ -187,7 +190,18 @@ cypress-run-all() {
fi
export CYPRESS_BASE_URL
- nohup flask run --no-debugger -p $port >"$flasklog" 2>&1 </dev/null &
+ nohup gunicorn \
+ --bind "127.0.0.1:$port" \
+ --workers 4 \
+ --worker-class gthread \
+ --threads 20 \
+ --timeout 120 \
+ --max-requests 500 \
+ --max-requests-jitter 50 \
+ --access-logfile - \
+ --error-logfile - \
+ "superset.app:create_app()" \
+ >"$flasklog" 2>&1 </dev/null &
local flaskProcessId=$!
Review Comment:
cypress-run-all() starts gunicorn in the background but never waits for it
to become healthy before launching Cypress. Unlike playwright-run() (which
polls `/health`), this can race and produce intermittent connection errors if
the first spec starts before the server is listening. Add a short readiness
loop (curl `/health` with a timeout) and fail fast with the startup log if the
backend never comes up.
##########
.github/workflows/bashlib.sh:
##########
@@ -175,9 +175,12 @@ cypress-run-all() {
local APP_ROOT=$2
cd "$GITHUB_WORKSPACE/superset-frontend/cypress-base"
- # Start Flask and run it in background
- # --no-debugger means disable the interactive debugger on the 500 page
- # so errors can print to stderr.
+ # Start the Superset backend via gunicorn (not `flask run`). The Flask
+ # development server is single-threaded and has no crash-recovery, so
+ # heavy tests (dashboard import/export, SQL Lab) can knock it offline
+ # for the rest of the run — surfacing as `ECONNREFUSED` / `socket hang up`
+ # / `Missing CSRF token` cascades. Gunicorn gives us multiple workers,
+ # a request timeout, and worker-recycling under load.
local flasklog="${HOME}/flask.log"
local port=8081
Review Comment:
Several identifiers/messages still refer to Flask even though the backend is
now started via gunicorn (e.g., `flasklog`, `flaskProcessId`, and the later
"Flask log" group label). Renaming these to backend/gunicorn-oriented names
will reduce confusion when debugging CI failures.
##########
.github/workflows/bashlib.sh:
##########
@@ -224,7 +238,9 @@ playwright-run() {
local APP_ROOT=$1
local TEST_PATH=$2
- # Start Flask from the project root (same as Cypress)
+ # Start the Superset backend via gunicorn from the project root.
+ # See cypress-run-all() above for the rationale — the Flask dev server
+ # cannot survive the dashboard import/export tests under load.
cd "$GITHUB_WORKSPACE"
local flasklog="${HOME}/flask-playwright.log"
local port=8081
Review Comment:
playwright-run() also continues to use `flasklog`/"Flask log" naming even
though the process is gunicorn. Consider renaming the log variable and group
titles so logs are easier to interpret during CI triage (especially since there
are now two different log files).
--
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]
