Skip to content

Refactor configuration usage, improve Dockerfile, and add startup validation & logging - #17

Merged
ThomasBury merged 2 commits into
mainfrom
codex/conduct-in-depth-code-review-for-best-practices-wsyulx
Mar 3, 2026
Merged

Refactor configuration usage, improve Dockerfile, and add startup validation & logging#17
ThomasBury merged 2 commits into
mainfrom
codex/conduct-in-depth-code-review-for-best-practices-wsyulx

Conversation

@ThomasBury

Copy link
Copy Markdown
Owner

Motivation

  • Centralize and extend runtime configuration to support logging, rate limits, and safe secret handling.
  • Harden the container image for better build caching, deterministic dependency install, and non-root runtime.
  • Validate configuration early at startup and use the settings consistently across auth, rate limiting, and logging.

Description

  • Updated Dockerfile to set PYTHONDONTWRITEBYTECODE/PYTHONUNBUFFERED, copy pyproject.toml and uv.lock first for layer caching, perform a two-stage uv sync to avoid installing the project twice, copy src afterwards, and add an unprivileged app user to run the image.
  • Expanded AppSettings in src/acebet/app/config.py with defaults for acebet_env, logging fields, and rate limits, added a redacted() helper to hide acebet_secret_key, and kept validate_config() to raise a clear RuntimeError on invalid settings.
  • Changed authentication in src/acebet/app/dependencies/auth.py to use the new settings object for JWT encoding/decoding and removed previous module-level constants, while preserving a development fallback secret.
  • Modified src/acebet/app/main.py to use an async lifespan that calls validate_config() at startup, wire Limiter defaults from settings, switch logging to settings.acebet_log_file and settings.acebet_log_level, and use settings.acebet_access_token_expire_minutes for token expiry.

Testing

  • Ran pytest and unit tests completed successfully.
  • Built the Docker image with docker build . which completed without errors.

Codex Task

@coderabbitai

coderabbitai Bot commented Mar 3, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@ThomasBury has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minutes and 52 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 17b0989 and 49a0492.

📒 Files selected for processing (4)
  • Dockerfile
  • src/acebet/app/config.py
  • src/acebet/app/dependencies/auth.py
  • src/acebet/app/main.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/conduct-in-depth-code-review-for-best-practices-wsyulx

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the application's robustness and maintainability by centralizing configuration management, improving the Docker build process for efficiency and security, and integrating these settings consistently across core functionalities like authentication, rate limiting, and logging. The changes aim to provide a more secure, predictable, and observable application environment.

Highlights

  • Configuration Refactoring: Centralized and extended runtime configuration using Pydantic's AppSettings to support logging, rate limits, and safe secret handling. A new redacted() helper method was added to prevent sensitive information from being logged.
  • Dockerfile Hardening and Optimization: Improved the Dockerfile for better build caching, deterministic dependency installation, and enhanced security by introducing a two-stage uv sync process and running the application as an unprivileged user.
  • Startup Validation and Logging: Implemented early configuration validation at application startup using an asynchronous lifespan context manager in FastAPI. Logging and rate limiting configurations are now consistently derived from the centralized AppSettings.
  • Authentication Module Update: Modified the authentication module to utilize the new settings object for JWT encoding and decoding, removing previous module-level constants and ensuring consistent configuration usage.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • Dockerfile
    • Added PYTHONDONTWRITEBYTECODE and PYTHONUNBUFFERED environment variables for Python runtime behavior.
    • Reordered COPY commands to optimize Docker layer caching by copying pyproject.toml and uv.lock first.
    • Implemented a two-stage uv sync process: first for dependencies without installing the project, then for the project itself.
    • Introduced an unprivileged app user and group, and changed the runtime user to app for enhanced security.
  • src/acebet/app/config.py
    • Imported Any from typing.
    • Set a default value of 'development' for acebet_env.
    • Added new configuration fields: acebet_log_level, acebet_log_file, acebet_default_rate_limit, and acebet_login_rate_limit with default values.
    • Adjusted the error message formatting in validate_expiry_minutes for readability.
    • Added a redacted() method to AppSettings to return a dictionary with the acebet_secret_key masked for safe logging.
    • Removed module-level constants ACEBET_SECRET_KEY, ACEBET_JWT_ALGORITHM, and ACEBET_ACCESS_TOKEN_EXPIRE_MINUTES.
  • src/acebet/app/dependencies/auth.py
    • Removed direct imports of ACEBET_ACCESS_TOKEN_EXPIRE_MINUTES, ACEBET_JWT_ALGORITHM, and ACEBET_SECRET_KEY.
    • Removed the ACCESS_TOKEN_EXPIRE_MINUTES module-level constant.
    • Updated jwt.encode to use settings.acebet_secret_key (with a development fallback) and settings.acebet_jwt_algorithm.
    • Updated jwt.decode to use settings.acebet_secret_key (with a development fallback) and settings.acebet_jwt_algorithm.
  • src/acebet/app/main.py
    • Imported asynccontextmanager from contextlib.
    • Imported the settings object from acebet.app.config.
    • Removed the import of ACCESS_TOKEN_EXPIRE_MINUTES.
    • Updated the limiter initialization to use settings.acebet_default_rate_limit.
    • Replaced the @app.on_event("startup") decorator with an asynccontextmanager lifespan function for startup configuration validation.
    • Updated logging.basicConfig to use settings.acebet_log_file and settings.acebet_log_level.
    • Updated the @limiter.limit decorator to use settings.acebet_login_rate_limit.
    • Updated the access_token_expires calculation to use settings.acebet_access_token_expire_minutes.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@ThomasBury
ThomasBury merged commit 7b122ee into main Mar 3, 2026
1 check passed

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request significantly improves the application's configuration management, Docker setup, and startup procedures. The move to a centralized settings object and the adoption of modern FastAPI features like the lifespan manager are excellent changes. My review includes one critical fix for the Dockerfile to ensure the container can start correctly with a non-root user, and a suggestion to improve maintainability in the authentication module by removing duplicated logic.

I am having trouble creating individual review comments. Click here to see my feedback.

Dockerfile (28)

critical

The container is configured to run as a non-privileged user app, which is an excellent security practice. However, the application is started on port 80, which is a privileged port. Non-root users cannot bind to ports below 1024, so this will cause the container to fail on startup. Please change this to a non-privileged port (e.g., 8080) and map the port during container runtime (e.g., docker run -p 80:8080 ...).

CMD ["/app/.venv/bin/fastapi", "run", "src/acebet/app/main.py", "--port", "8080", "--host", "0.0.0.0"]

src/acebet/app/dependencies/auth.py (123-127)

medium

The logic for providing a fallback secret key (settings.acebet_secret_key or "acebet-dev-insecure-secret-key") is duplicated here and in the get_current_user function (lines 149-153). This violates the DRY (Don't Repeat Yourself) principle and could lead to inconsistencies if one location is updated but the other is not.

To improve maintainability, this logic should be centralized. For example, you could define module-level constants at the top of the file:

SECRET_KEY = settings.acebet_secret_key or "acebet-dev-insecure-secret-key"
ALGORITHM = settings.acebet_jwt_algorithm

And then use these constants in both create_access_token and get_current_user.

@ThomasBury
ThomasBury deleted the codex/conduct-in-depth-code-review-for-best-practices-wsyulx branch April 29, 2026 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant