Skip to content

Fix backend detection in testing runners - #3944

Open
cyphercodes wants to merge 2 commits into
plotly:devfrom
cyphercodes:fix-3942-runner-backend-detection
Open

Fix backend detection in testing runners#3944
cyphercodes wants to merge 2 commits into
plotly:devfrom
cyphercodes:fix-3942-runner-backend-detection

Conversation

@cyphercodes

Copy link
Copy Markdown

Fixes #3942.

ThreadedRunner and MultiProcessRunner determined whether to pass Flask's threaded=True option by inspecting the concrete server class module. That misclassified wrapped or subclassed FastAPI/Quart servers whose classes are defined elsewhere.

Use Dash's already-resolved app.backend.server_type instead, while preserving the Flask fallback. The regression coverage includes a wrapped FastAPI-shaped server and the FastAPI, Quart, and Flask option paths.

Contributor Checklist

  • I have broken down my PR scope into the following TODO tasks
    • Detect runner behavior from the resolved Dash backend
    • Add regression coverage for wrapped ASGI servers and Flask fallback
  • I have run the tests locally and they passed. (refer to testing section in contributing)
  • I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR

optionals

  • I have added entry in the CHANGELOG.md
  • If this PR needs a follow-up in dash docs, community thread, I have mentioned the relevant URLS as follows
    • this GitHub #PR number updates the dash docs
    • here is the show and tell thread in Plotly Dash community

Local verification

  • python -m pytest tests/unit/test_app_runners.py -k 'wrapped_fastapi or uses_backend_type' -q — 4 passed
  • python -m pytest tests/unit/test_testing.py -q — 1 passed
  • Black, Flake8, Pylint, Pyright (production file), and git diff --check

@sonarqubecloud

Copy link
Copy Markdown

@T4rk1n T4rk1n left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, just some minor comments and need to add a changelog entry.

Comment on lines +32 to +36
server_type = getattr(getattr(app, "backend", None), "server_type", "flask")
if server_type in ("fastapi", "quart"):
app.run(**options)
else:
app.run(threaded=True, **options)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We lost the comments indicating that we need the threaded=True for flask testing.



def _run_app(app, options):
server_type = getattr(getattr(app, "backend", None), "server_type", "flask")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is duplicated code at line 221-222, could potentially extract that into a _server_type function to prevent drift.

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

Labels

None yet

Projects

None yet

2 participants