Add minimal runnable BHV app skeleton with lifecycle hooks #60
Conversation
Summary of ChangesHello @Sahil-u07, 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 establishes the foundational structure for a new BHV application. It sets up a minimal, runnable Flask web server, defining key architectural placeholders and integrating essential Flask lifecycle hooks. The primary goal is to provide a basic execution flow, allowing for subsequent feature development on a stable base. Highlights
🧠 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. Using Gemini Code AssistThe 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
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 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request provides a good starting point for the BHV application by setting up a minimal Flask skeleton. My review includes several suggestions to improve maintainability and future-proofing. The main points are:
- Adherence to the project's contribution guidelines by adding missing docstrings to all new classes and functions.
- Replacing the deprecated
@app.before_first_requestdecorator with a more modern alternative to avoid issues with future Flask versions.
These changes will make the initial codebase more robust and easier for other contributors to build upon.
| @app.before_first_request | ||
| def before_first_request(): | ||
| pass |
There was a problem hiding this comment.
The @app.before_first_request decorator is deprecated as of Flask 2.3 and will be removed in Flask 2.4. It's recommended to use app.before_request instead. Note that before_request runs before every request. For one-time setup, you can perform it before app.run() or use a flag within before_request to ensure the setup code runs only once.
| @app.before_first_request | |
| def before_first_request(): | |
| pass | |
| @app.before_request | |
| def before_request(): | |
| pass |
| class Database: | ||
| def __init__(self): | ||
| self.connection = None | ||
|
|
||
| def connect(self): | ||
| pass | ||
|
|
||
| def disconnect(self): | ||
| pass |
There was a problem hiding this comment.
Per the CONTRIBUTING.md style guide (line 359), all classes and functions should have docstrings. The Database class and its methods are missing them. Adding docstrings will improve code clarity and maintainability.
| class Database: | |
| def __init__(self): | |
| self.connection = None | |
| def connect(self): | |
| pass | |
| def disconnect(self): | |
| pass | |
| class Database: | |
| """Manages the database connection and operations.""" | |
| def __init__(self): | |
| """Initialize the database.""" | |
| self.connection = None | |
| def connect(self): | |
| """Establish a connection to the database.""" | |
| pass | |
| def disconnect(self): | |
| """Disconnect from the database.""" | |
| pass |
References
- Add docstrings to functions and classes (link)
| class Auth: | ||
| def __init__(self): | ||
| pass | ||
|
|
||
| def verify(self, token): | ||
| pass |
There was a problem hiding this comment.
Per the CONTRIBUTING.md style guide (line 359), all classes and functions should have docstrings. The Auth class and its methods are missing them. Adding docstrings will improve code clarity and maintainability.
| class Auth: | |
| def __init__(self): | |
| pass | |
| def verify(self, token): | |
| pass | |
| class Auth: | |
| """Handles authentication and token verification.""" | |
| def __init__(self): | |
| """Initialize the authentication handler.""" | |
| pass | |
| def verify(self, token): | |
| """Verify the provided authentication token.""" | |
| pass |
References
- Add docstrings to functions and classes (link)
| @app.teardown_appcontext | ||
| def teardown_appcontext(exception): | ||
| pass |
There was a problem hiding this comment.
Per the CONTRIBUTING.md style guide (line 359), all functions should have docstrings. Please add a docstring to teardown_appcontext to explain its role in cleaning up resources after a request.
| @app.teardown_appcontext | |
| def teardown_appcontext(exception): | |
| pass | |
| @app.teardown_appcontext | |
| def teardown_appcontext(exception): | |
| """Clean up resources at the end of a request.""" | |
| pass |
References
- Add docstrings to functions and classes (link)
| @app.route('/') | ||
| def index(): | ||
| return 'BHV is running' |
There was a problem hiding this comment.
Per the CONTRIBUTING.md style guide (line 359), all functions should have docstrings. Please add a docstring to the index view function to describe its purpose.
| @app.route('/') | |
| def index(): | |
| return 'BHV is running' | |
| @app.route('/') | |
| def index(): | |
| """Index route to confirm the application is running.""" | |
| return 'BHV is running' |
References
- Add docstrings to functions and classes (link)
There was a problem hiding this comment.
Pull request overview
Adds a minimal Flask-based BHV application entrypoint intended to be runnable and establish basic lifecycle structure.
Changes:
- Introduces a new
app.pyFlask application with a/health route - Adds placeholder
DatabaseandAuthclasses for future integration - Adds Flask lifecycle hook stubs for startup/teardown wiring
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| db = Database() | ||
| auth = Auth() | ||
|
|
||
| @app.before_first_request |
There was a problem hiding this comment.
@app.before_first_request is deprecated in newer Flask releases and may break when the project upgrades. Prefer @app.before_serving (or an app-factory setup step) for one-time startup initialization.
| @app.before_first_request | |
| @app.before_serving |
| auth = Auth() | ||
|
|
||
| @app.before_first_request | ||
| def before_first_request(): |
There was a problem hiding this comment.
The function name before_first_request shadows the lifecycle concept and reads like the decorator itself; consider a clearer name like init_app()/startup() to make stack traces and logs easier to interpret.
| def before_first_request(): | |
| def init_app(): |
| @app.teardown_appcontext | ||
| def teardown_appcontext(exception): | ||
| pass |
There was a problem hiding this comment.
teardown_appcontext currently no-ops, so any future resources (e.g., database connections) won’t be cleaned up. If the intent is to establish lifecycle flow, wire this to db.disconnect() (and/or rollback/close) and use the passed exception as needed.
| app = Flask(__name__) | ||
|
|
||
| class Database: | ||
| def __init__(self): |
There was a problem hiding this comment.
PEP 8 expects two blank lines between top-level definitions (module constants/assignments, classes, and functions). Add an extra blank line before class Database (and similarly between other top-level blocks) to keep formatting consistent with standard Python tooling.
|
@Sahil-u07, make sure you raise PR against the |
3 participants
Adds a minimal, runnable application skeleton for BHV, focused on establishing
basic execution flow without implementing features.