Conversation
## Walkthrough
This update modifies the server startup process by introducing a Gunicorn configuration file and changing the Dockerfile and entrypoint script to launch the FastAPI application using Gunicorn via the `uv` tool instead of `uvicorn` and a shell script. It adds dependencies for Gunicorn and its Uvicorn worker, removes the `DEVELOPMENT` environment variable from the example Docker Compose file, and refactors utility logic for repository and webhook settings into a reusable function. A new server startup hook is added to initialize repository and webhook settings on server launch.
## Changes
| Files/Paths | Change Summary |
|-------------------------------------------------|-------------------------------------------------------------------------------------------------------------|
| Dockerfile, entrypoint.sh, gunicorn.conf.py | Switched server startup from `uvicorn` via shell script to `gunicorn` via `uv` with config file; added Gunicorn config; removed entrypoint.sh script. |
| pyproject.toml | Added dependencies: `gunicorn` and `uvicorn-worker`. |
| docker-compose-example.yaml | Removed `DEVELOPMENT=false` environment variable; added `WEBHOOK_SERVER_IP_BIND` and `WEBHOOK_SERVER_PORT`. |
| webhook_server/app.py | Added `on_starting(server)` function to run repository and webhook setup at server startup. |
| webhook_server/utils/github_repository_and_webhook_settings.py | Refactored main execution logic into new `repository_and_webhook_settings()` function for reuse. |
| webhook_server/libs/github_api.py | Modified check run methods to re-run checks even if previously marked in progress, updating log messages accordingly. |
## Possibly related PRs
- myk-org/github-webhook-server#579: Both PRs modify the Dockerfile and entrypoint.sh to switch the server startup mechanism to use `uv` and Gunicorn, focusing on replacing `poetry` with `uv` for running the FastAPI app.
## Suggested labels
`verified`
## Suggested reviewers
- rnetser📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Report bugs in Issues The following are automatically added:
Available user actions:
PR will be approved when the following conditions are met:
Approvers and Reviewers
Supported /retest check runs
Supported labels
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Dockerfile (1)
37-38: Remove unusedentrypoint.shfrom build artifactsThe Dockerfile still copies
entrypoint.shinto the image but never uses it (theENTRYPOINTnow directly invokes Gunicorn). This redundant file increases image size and maintenance overhead. Consider removingentrypoint.shfrom theCOPYdirective or deleting the file altogether.entrypoint.sh (1)
3-3: Remove or utilize the unusedSERVER_RUN_CMDvariableThe
SERVER_RUN_CMDvariable is assigned but never referenced or exported (SC2034). Since the Dockerfile no longer uses this script as an entrypoint, consider deletingentrypoint.shentirely or updating the script to actually execute"$SERVER_RUN_CMD".🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: SERVER_RUN_CMD appears unused. Verify use (or export if used externally).
(SC2034)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
Dockerfile(2 hunks)docker-compose-example.yaml(0 hunks)entrypoint.sh(1 hunks)gunicorn.conf.py(1 hunks)pyproject.toml(1 hunks)webhook_server/app.py(1 hunks)webhook_server/utils/github_repository_and_webhook_settings.py(2 hunks)
💤 Files with no reviewable changes (1)
- docker-compose-example.yaml
🧰 Additional context used
🧬 Code Graph Analysis (1)
gunicorn.conf.py (1)
webhook_server/app.py (1)
on_starting(18-19)
🪛 Shellcheck (0.10.0)
entrypoint.sh
[warning] 3-3: SERVER_RUN_CMD appears unused. Verify use (or export if used externally).
(SC2034)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pre-commit
- GitHub Check: python-module-install
- GitHub Check: build-container
- GitHub Check: tox
🔇 Additional comments (7)
pyproject.toml (1)
69-70: Dependencies added for gunicorn support.The PR adds two new dependencies to support the migration from uvicorn to gunicorn:
uvicorn-worker>=0.3.0- Provides the Uvicorn worker class for Gunicorngunicorn>=23.0.0- The Gunicorn server itselfThese are appropriate additions that align with the PR objective to leverage Gunicorn's
on_startinghook.webhook_server/utils/github_repository_and_webhook_settings.py (2)
21-46: Good refactoring to support server startup hook.The code that was previously under
if __name__ == "__main__":has been effectively extracted into a reusable function. This allows it to be called during server startup via Gunicorn'son_startinghook while preserving the ability to run it directly.The function initializes logging, loads configuration, retrieves GitHub API instances, and performs the necessary repository settings and webhook creation operations.
48-49: Clean update to main block.The main block now simply calls the refactored function, maintaining the original behavior when the script is run directly.
gunicorn.conf.py (1)
1-7: Well-structured Gunicorn configuration.The configuration:
- Binds to all interfaces (0.0.0.0) on port 5000
- Uses Uvicorn's ASGI worker for FastAPI compatibility
- Sets worker count configurable via environment variable
- Configures the
on_startinghook to initialize repository and webhook settings- Directs access logs to stdout for container log capture
This configuration properly implements the PR objective of using Gunicorn's startup hook.
webhook_server/app.py (2)
10-10: New import for repository and webhook settings.The import statement correctly references the refactored function that will be used in the
on_startinghook.
18-20: Added Gunicorn startup hook function.This function implements the core objective of the PR - using Gunicorn's
on_startinghook to initialize repository and webhook settings before the server fully starts. The function signature matches what Gunicorn expects for this hook.Dockerfile (1)
69-69:Details
✅ Verification successful
Verify Gunicorn bind configuration matches exposed port
The
ENTRYPOINTinvokes Gunicorn with your custom config, but we need to ensuregunicorn.conf.pybinds to0.0.0.0:5000(matchingEXPOSE 5000). Without this, the container may not accept external connections.Run the following script from the repo root to confirm the bind setting:
🏁 Script executed:
#!/bin/bash # Check the bind address in gunicorn.conf.py grep -R "bind\s*=" gunicorn.conf.pyLength of output: 59
Gunicorn bind configuration matches EXPOSE port
Thebind = "0.0.0.0:5000"setting ingunicorn.conf.pyaligns withEXPOSE 5000in the Dockerfile. No changes are needed.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
webhook_server/libs/github_api.py (4)
1332-1334: Ensure consistent logging level for re-running checks.This newly added check for in-progress build container checks uses
infolevel logging, while similar checks for TOX and pre-commit usedebuglevel logging. Consider standardizing the log level across all similar check run messages.- self.logger.info(f"{self.log_prefix} Check run is in progress, re-running {BUILD_CONTAINER_STR}.") + self.logger.debug(f"{self.log_prefix} Check run is in progress, re-running {BUILD_CONTAINER_STR}.")
1340-1340: Ensure consistent logging level for re-running checks.Similar to the above, this log message uses
infolevel while other similar checks usedebug. Consider standardizing for consistency.- self.logger.info(f"{self.log_prefix} Check run is in progress, re-running {BUILD_CONTAINER_STR}.") + self.logger.debug(f"{self.log_prefix} Check run is in progress, re-running {BUILD_CONTAINER_STR}.")
1421-1421: Ensure consistent logging level for re-running checks.This log message uses
infolevel while similar checks for TOX and pre-commit usedebug. Consider standardizing for consistency.- self.logger.info(f"{self.log_prefix} Check run is in progress, re-running {PYTHON_MODULE_INSTALL_STR}.") + self.logger.debug(f"{self.log_prefix} Check run is in progress, re-running {PYTHON_MODULE_INSTALL_STR}.")
2175-2177: Ensure consistent logging level for re-running checks.The log message for conventional title check re-run uses
infolevel while similar checks for TOX and pre-commit usedebug. Consider standardizing for consistency.- self.logger.info(f"{self.log_prefix} Check run is in progress, re-running {CONVENTIONAL_TITLE_STR}.") + self.logger.debug(f"{self.log_prefix} Check run is in progress, re-running {CONVENTIONAL_TITLE_STR}.")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
gunicorn.conf.py(1 hunks)webhook_server/libs/github_api.py(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- gunicorn.conf.py
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: python-module-install
- GitHub Check: build-container
- GitHub Check: pre-commit
- GitHub Check: tox
🔇 Additional comments (2)
webhook_server/libs/github_api.py (2)
960-960: Changed log message reflects new behavior of re-running checks instead of skipping.The logging message now correctly indicates that the check is being re-run even when it's already in progress, which aligns with the new behavior of not returning early from the function.
995-995: Changed log message reflects new behavior of re-running checks instead of skipping.Similar to the TOX check, this log message has been updated to reflect that the pre-commit check will be re-run rather than skipped when already in progress.
|
/verified |
2 participants
Switch to
gunicornfromuvicorn.gunicornsupporton_startinghook which can run code before the server starts.Summary by CodeRabbit
New Features
Refactor
Chores