Replace Gunicorn with Granian to improve pgAdmin 4 performance#9786
Replace Gunicorn with Granian to improve pgAdmin 4 performance#9786Smartappli wants to merge 3 commits intopgadmin-org:masterfrom
Conversation
Updated granian package version from 2.2.5 to 2.7.2 in Dockerfile.
…anian Replace Gunicorn with Granian in Docker runtime
WalkthroughThe changes replace the Gunicorn application server with Granian in the PgAdmin Docker setup. The Dockerfile now installs Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/docker/entrypoint.sh (2)
204-204: Environment variable naming: Consider documenting backward compatibility.The script retains
GUNICORN_THREADSandGUNICORN_ACCESS_LOGFILEenvironment variable names for backward compatibility, which is good. However, users may be confused by Gunicorn-prefixed variables when running Granian.Consider adding a comment explaining this is for backward compatibility, or adding support for new
GRANIAN_*aliases in the future.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/docker/entrypoint.sh` at line 204, Add a short inline comment above the exec line explaining that GUNICORN_* vars (e.g., GUNICORN_THREADS and GUNICORN_ACCESS_LOGFILE) are kept for backward compatibility when running Granian, and optionally add support for GRanian-prefixed aliases by reading GRANIAN_THREADS and GRANIAN_ACCESS_LOGFILE (fallback to GUNICORN_* if unset) before composing ACCESS_LOG_ARGS, TLS_ARGS, and BIND_ARGS so users can use either naming convention.
141-143: Stale comment references Gunicorn.The comment on line 141 still mentions "Gunicorn" but the server has been changed to Granian.
📝 Suggested fix
- # Initialize DB before starting Gunicorn + # Initialize DB before starting Granian # Importing pgadmin4 (from this script) is enough /venv/bin/python3 run_pgadmin.py🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/docker/entrypoint.sh` around lines 141 - 143, Update the stale comment that mentions "Gunicorn" to reference the current server "Granian" instead; locate the comment above the run_pgadmin.py invocation (the line commenting DB initialization) and change the wording from "Initialize DB before starting Gunicorn" to something like "Initialize DB before starting Granian" so the comment matches the actual server being used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/docker/entrypoint.sh`:
- Around line 192-196: The current branch only enables access logs when
GUNICORN_ACCESS_LOGFILE equals "-" which breaks support for file paths; update
the logic around the GUNICORN_ACCESS_LOGFILE check so it handles three cases:
"-" -> set ACCESS_LOG_ARGS to enable logging to stdout (e.g., "--access-log"),
empty/unset -> set ACCESS_LOG_ARGS to disable access logging (e.g.,
"--no-access-log"), and any other non-empty value -> treat it as a file path by
setting ACCESS_LOG_ARGS to the flag that enables logging to a file (e.g.,
including the file path) and emit a warning via the existing logger/echo
indicating a custom file path is being used; reference the
GUNICORN_ACCESS_LOGFILE env var and ACCESS_LOG_ARGS variable when making this
change.
---
Nitpick comments:
In `@pkg/docker/entrypoint.sh`:
- Line 204: Add a short inline comment above the exec line explaining that
GUNICORN_* vars (e.g., GUNICORN_THREADS and GUNICORN_ACCESS_LOGFILE) are kept
for backward compatibility when running Granian, and optionally add support for
GRanian-prefixed aliases by reading GRANIAN_THREADS and GRANIAN_ACCESS_LOGFILE
(fallback to GUNICORN_* if unset) before composing ACCESS_LOG_ARGS, TLS_ARGS,
and BIND_ARGS so users can use either naming convention.
- Around line 141-143: Update the stale comment that mentions "Gunicorn" to
reference the current server "Granian" instead; locate the comment above the
run_pgadmin.py invocation (the line commenting DB initialization) and change the
wording from "Initialize DB before starting Gunicorn" to something like
"Initialize DB before starting Granian" so the comment matches the actual server
being used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 60c8a8bb-2fcb-494b-8259-8f4f362199d9
📒 Files selected for processing (3)
Dockerfilepkg/docker/entrypoint.shpkg/docker/gunicorn_config.py
💤 Files with no reviewable changes (1)
- pkg/docker/gunicorn_config.py
This change proposes replacing Gunicorn with Granian as the application server for pgAdmin 4 in order to improve overall performance, scalability, and efficiency.
Granian offers a modern Rust-based runtime with strong ASGI/WSGI support, lower overhead, and better concurrency handling compared to Gunicorn. For pgAdmin 4, this could lead to:
The goal of this change is to modernize the deployment stack while maintaining compatibility with the existing pgAdmin 4 application architecture.
Further benchmarking and validation may be required to confirm gains across different deployment environments.
Summary by CodeRabbit