-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor Flask app initialization and add logging #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Refactor app initialization into a factory function and configure logging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the Flask application initialization to use the application factory pattern, improving testability and configuration management. It also adds comprehensive logging and better error handling.
- Introduces a
create_app()factory function that allows for dependency injection of configuration - Adds logging throughout the application lifecycle with proper error handling
- Makes the application more configurable via environment variables (PORT, FLASK_ENV, FLASK_DEBUG)
| base_dir: str = os.path.abspath(os.path.dirname(__file__)) | ||
|
|
||
| app: Flask = Flask(__name__) | ||
| def create_app(config: dict = None) -> Flask: |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
El parámetro config debería usar dict[str, Any] o dict | None en lugar de dict para mayor precisión en el tipo. Además, considera usar Optional[dict[str, Any]] para mayor claridad sobre que puede ser None.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new pull request to apply changes based on this feedback
| logger = logging.getLogger(__name__) | ||
|
|
||
| # Get the server directory path | ||
| base_dir: str = os.path.abspath(os.path.dirname(__file__)) |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
La variable base_dir se define pero nunca se utiliza en el archivo. Considera eliminarla o úsala si tiene un propósito específico.
| base_dir: str = os.path.abspath(os.path.dirname(__file__)) |
| host='0.0.0.0' # Allow external connections | ||
| ) | ||
| except ValueError: | ||
| logger.error("Invalid PORT environment variable") |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
El mensaje de error no incluye el valor inválido que causó el problema. Considera cambiar a logger.error(f"Invalid PORT environment variable: {os.getenv('PORT')}") para facilitar la depuración.
| logger.error("Invalid PORT environment variable") | |
| logger.error(f"Invalid PORT environment variable: {os.getenv('PORT')}") |
|
@chikamsoachumsft I've opened a new pull request, #27, to work on those changes. Once the pull request is ready, I'll request review from you. |
Refactor app initialization into a factory function and configure logging.