-
Notifications
You must be signed in to change notification settings - Fork 1
Feature/app factory pattern #27
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
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
cb87636
move to app factory pattern, using dedicated configs
piyush-jaiswal b80cdeb
no need for app context as already pushed with client in setup fixture
piyush-jaiswal 7db3923
missing db param
piyush-jaiswal e15800e
typo
piyush-jaiswal a2fe147
remove redundant app context as already pushed during setup
piyush-jaiswal e22acd1
remove unneeded lambda param
piyush-jaiswal File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,5 @@ | ||
| from app import app | ||
| from app import create_app | ||
| from config import ProductionConfig | ||
|
|
||
|
|
||
| app = create_app(ProductionConfig) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,99 +1,28 @@ | ||
| import os | ||
| from datetime import timedelta | ||
| from flask import Flask | ||
|
|
||
| from flask import Flask, jsonify | ||
| from flask_jwt_extended import JWTManager | ||
| from flask_migrate import Migrate | ||
| from flask_sqlalchemy import SQLAlchemy | ||
| from dotenv import load_dotenv | ||
| from sqlalchemy import MetaData | ||
| from flask_smorest import Api | ||
| from app.extensions import api, db, jwt, migrate | ||
| from config import DevelopmentConfig | ||
|
|
||
|
|
||
| def register_blueprints(): | ||
| def create_app(config_class=DevelopmentConfig): | ||
| app = Flask(__name__) | ||
| app.config.from_object(config_class) | ||
|
|
||
| # initialize extensions | ||
| db.init_app(app) | ||
| migrate.init_app(app, db) | ||
| jwt.init_app(app) | ||
| api.init_app(app) | ||
|
|
||
| # register blueprints | ||
| from app.routes.auth import bp as auth_bp | ||
| from app.routes.category import bp as category_bp | ||
| from app.routes.subcategory import bp as subcategory_bp | ||
| from app.routes.product import bp as product_bp | ||
| from app.routes.auth import bp as auth_bp | ||
| from app.routes.subcategory import bp as subcategory_bp | ||
|
|
||
| api.register_blueprint(category_bp, url_prefix="/categories") | ||
| api.register_blueprint(subcategory_bp, url_prefix="/subcategories") | ||
| api.register_blueprint(product_bp, url_prefix="/products") | ||
| api.register_blueprint(auth_bp, url_prefix="/auth") | ||
|
|
||
|
|
||
| app = Flask(__name__) | ||
|
|
||
| load_dotenv() | ||
|
|
||
| # sqlalchemy | ||
| app.config['SQLALCHEMY_DATABASE_URI'] = os.getenv("SQLALCHEMY_DATABASE_URI") | ||
| app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False | ||
|
|
||
| # jwt | ||
| app.config["JWT_SECRET_KEY"] = os.getenv("JWT_SECRET_KEY") | ||
| app.config["JWT_ACCESS_TOKEN_EXPIRES"] = timedelta(hours=3) | ||
| app.config["JWT_REFRESH_TOKEN_EXPIRES"] = timedelta(days=3) | ||
|
|
||
| # flask-smorest | ||
| app.config["API_TITLE"] = "Ecommerce REST API" | ||
| app.config["API_VERSION"] = "v1" | ||
| app.config["OPENAPI_VERSION"] = "3.0.2" | ||
|
|
||
| # flask-smorest openapi swagger | ||
| app.config["OPENAPI_URL_PREFIX"] = "/" | ||
| app.config["OPENAPI_SWAGGER_UI_PATH"] = "/" | ||
| app.config["OPENAPI_SWAGGER_UI_URL"] = "https://cdn.jsdelivr.net/npm/swagger-ui-dist/" | ||
|
|
||
| # flask-smorest Swagger UI top level authorize dialog box | ||
| app.config["API_SPEC_OPTIONS"] = { | ||
| "components": { | ||
| "securitySchemes": { | ||
| "access_token": { | ||
| "type": "http", | ||
| "scheme": "bearer", | ||
| "bearerFormat": "JWT", | ||
| "description": "Enter your JWT access token", | ||
| }, | ||
| "refresh_token": { | ||
| "type": "http", | ||
| "scheme": "bearer", | ||
| "bearerFormat": "JWT", | ||
| "description": "Enter your JWT refresh token", | ||
| }, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| # PostgreSQL-compatible naming convention (to follow the naming convention already used in the DB) | ||
| # https://stackoverflow.com/questions/4107915/postgresql-default-constraint-names | ||
| naming_convention = { | ||
| "ix": "%(table_name)s_%(column_0_name)s_idx", # Indexes | ||
| "uq": "%(table_name)s_%(column_0_name)s_key", # Unique constraints | ||
| "ck": "%(table_name)s_%(constraint_name)s_check", # Check constraints | ||
| "fk": "%(table_name)s_%(column_0_name)s_fkey", # Foreign keys | ||
| "pk": "%(table_name)s_pkey" # Primary keys | ||
| } | ||
| metadata = MetaData(naming_convention=naming_convention) | ||
| db = SQLAlchemy(app, metadata=metadata) | ||
| migrate = Migrate(app, db) | ||
| jwt = JWTManager(app) | ||
| api = Api(app) | ||
|
|
||
| register_blueprints() | ||
|
|
||
|
|
||
| @jwt.expired_token_loader | ||
| def expired_token_callback(jwt_header, jwt_payload): | ||
| err = "Access token expired. Use your refresh token to get a new one." | ||
| if jwt_payload['type'] == 'refresh': | ||
| err = "Refresh token expired. Please login again." | ||
| return jsonify(code="token_expired", error=err), 401 | ||
|
|
||
| @jwt.invalid_token_loader | ||
| def invalid_token_callback(error): | ||
| return jsonify(code="invalid_token", error="Invalid token provided."), 401 | ||
|
|
||
| @jwt.unauthorized_loader | ||
| def missing_token_callback(error): | ||
| return jsonify(code="authorization_required", error="JWT needed for this operation. Login, if needed."), 401 | ||
| return app |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| from flask import jsonify | ||
| from flask_jwt_extended import JWTManager | ||
| from flask_migrate import Migrate | ||
| from flask_smorest import Api | ||
| from flask_sqlalchemy import SQLAlchemy | ||
| from sqlalchemy import MetaData | ||
|
|
||
|
|
||
| # PostgreSQL-compatible naming convention (to follow the naming convention already used in the DB) | ||
| # https://stackoverflow.com/questions/4107915/postgresql-default-constraint-names | ||
| naming_convention = { | ||
| "ix": "%(table_name)s_%(column_0_name)s_idx", # Indexes | ||
| "uq": "%(table_name)s_%(column_0_name)s_key", # Unique constraints | ||
| "ck": "%(table_name)s_%(constraint_name)s_check", # Check constraints | ||
| "fk": "%(table_name)s_%(column_0_name)s_fkey", # Foreign keys | ||
| "pk": "%(table_name)s_pkey", # Primary keys | ||
| } | ||
| metadata = MetaData(naming_convention=naming_convention) | ||
| db = SQLAlchemy(metadata=metadata) | ||
| migrate = Migrate(db) | ||
| jwt = JWTManager() | ||
| api = Api() | ||
|
|
||
|
|
||
| @jwt.expired_token_loader | ||
| def expired_token_callback(jwt_header, jwt_payload): | ||
| err = "Access token expired. Use your refresh token to get a new one." | ||
| if jwt_payload["type"] == "refresh": | ||
| err = "Refresh token expired. Please login again." | ||
| return jsonify(code="token_expired", error=err), 401 | ||
|
|
||
|
|
||
| @jwt.invalid_token_loader | ||
| def invalid_token_callback(error): | ||
| return jsonify(code="invalid_token", error="Invalid token provided."), 401 | ||
|
|
||
|
|
||
| @jwt.unauthorized_loader | ||
| def missing_token_callback(error): | ||
| return jsonify( | ||
| code="authorization_required", | ||
| error="JWT needed for this operation. Login, if needed.", | ||
| ), 401 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| import os | ||
| from datetime import timedelta | ||
|
|
||
| from dotenv import load_dotenv | ||
|
|
||
|
|
||
| load_dotenv() | ||
|
|
||
|
|
||
| class Config: | ||
| # sqlalchemy | ||
| SQLALCHEMY_TRACK_MODIFICATIONS = False | ||
|
|
||
| # jwt | ||
| JWT_SECRET_KEY = os.getenv("JWT_SECRET_KEY") | ||
| JWT_ACCESS_TOKEN_EXPIRES = timedelta(hours=3) | ||
| JWT_REFRESH_TOKEN_EXPIRES = timedelta(days=3) | ||
|
|
||
| # flask-smorest | ||
| API_TITLE = "Ecommerce REST API" | ||
| API_VERSION = "v1" | ||
| OPENAPI_VERSION = "3.0.2" | ||
|
|
||
| # flask-smorest openapi swagger | ||
| OPENAPI_URL_PREFIX = "/" | ||
| OPENAPI_SWAGGER_UI_PATH = "/" | ||
| OPENAPI_SWAGGER_UI_URL = "https://cdn.jsdelivr.net/npm/swagger-ui-dist/" | ||
|
|
||
| # flask-smorest Swagger UI top level authorize dialog box | ||
| API_SPEC_OPTIONS = { | ||
| "components": { | ||
| "securitySchemes": { | ||
| "access_token": { | ||
| "type": "http", | ||
| "scheme": "bearer", | ||
| "bearerFormat": "JWT", | ||
| "description": "Enter your JWT access token", | ||
| }, | ||
| "refresh_token": { | ||
| "type": "http", | ||
| "scheme": "bearer", | ||
| "bearerFormat": "JWT", | ||
| "description": "Enter your JWT refresh token", | ||
| }, | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
| class DevelopmentConfig(Config): | ||
| DEBUG = True | ||
| SQLALCHEMY_DATABASE_URI = os.getenv("SQLALCHEMY_DATABASE_URI") | ||
|
|
||
|
|
||
| class TestingConfig(Config): | ||
| TESTING = True | ||
| SQLALCHEMY_DATABASE_URI = "sqlite:///:memory:" | ||
| JWT_SECRET_KEY = os.urandom(24).hex() | ||
|
|
||
|
|
||
| class ProductionConfig(Config): | ||
| SQLALCHEMY_DATABASE_URI = os.getenv("SQLALCHEMY_DATABASE_URI") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
In‑memory SQLite can reset across connections
Using sqlite:///:memory: without StaticPool may create a fresh DB per connection, causing flaky tests (tables “disappear” between create_all and requests). Use StaticPool and disable same thread checks, or use a file DB.
Update TestingConfig (in config.py), and import StaticPool:
Alternatively: use a temp file URI (e.g., sqlite:///test.db) and remove StaticPool.
📝 Committable suggestion
🤖 Prompt for AI Agents
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.
SqlAlchemy with Sqlite by default uses SingletonThreadPool, which maintains a single connection per thread. My tests run in a single thread sequentially and each test creates and drops all tables and data as its setup to ensure test isolation. All my tests pass. So, I don't think StaticPool with "check_same_thread": False is needed currently.
With the type of tests that i am writing won't using a single connection across multiple threads be a problem, as i am checking the number of categories created, and i assume i am the only one that is creating it. what if another thread also creates categories and my assertion of number of categories created in the table fails?
What do you think?
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.
@piyush-jaiswal, you're absolutely correct! Your analysis is spot-on:
Your current setup is appropriate for sequential single-threaded tests:
✅ SingletonThreadPool is sufficient: Since pytest runs tests sequentially by default (single thread), the SingletonThreadPool maintains one connection throughout the test session, keeping the in-memory database intact.
✅ Test isolation: Your setup/teardown with
db.create_all()anddb.drop_all()in theappfixture ensures each test starts with a clean slate, which is the right approach.✅ Your threading concern is valid: You're right that with StaticPool +
check_same_thread: False, if tests were running in parallel threads, you could have race conditions where multiple threads create categories simultaneously, breaking your count assertions.When StaticPool would be needed:
pytest-xdist(pytest -n auto)Since all your tests pass and you're running sequentially, no changes are needed. Your current configuration is correct for your use case.
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.
for reference later: https://docs.sqlalchemy.org/en/20/dialects/sqlite.html#threading-pooling-behavior
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.
@piyush-jaiswal, thanks for sharing that reference! It's a great resource to have on hand for understanding SQLite's pooling behavior in different scenarios. I've noted it for future reference.
✏️ Learnings added