-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
Binding DB sessions based on SQLAlchemy 1, changing how to declare Base Model classes, and other code modernization #5
base: master
Are you sure you want to change the base?
Conversation
…se Model classes, and other code modernization - DB session binding based on SQLAlchemy 2, Base Model class declaration method change - Reflected select, delete code based on SQLAlchemy 2 - Changed how to declare Model class based on SQLAlchemy 2 - Fixed import errors found in several examples - Added pyproject.toml file after removing setup.py due to the introduction of PEP 517/518 - Fixed minimum installed version to Python 3.7
Warning Rate Limit Exceeded@pre-commit-ci[bot] has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 38 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. WalkthroughThe updates involve making the codebase compatible with SQLAlchemy version 2, adopting new Python packaging standards as defined by PEP 517 and PEP 518, and ensuring the minimum Python version is set to 3.7. Additionally, there's a refactoring of the base class for SQLAlchemy models in the Changes
Related issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
pyproject.toml
is excluded by:!**/*.toml
setup.cfg
is excluded by:!**/*.cfg
Files selected for processing (3)
- CHANGELOG.md (1 hunks)
- Makefile (1 hunks)
- social_flask_sqlalchemy/models.py (4 hunks)
Additional comments: 5
Makefile (2)
- 2-2: The use of
python -m build
is a modern and recommended approach for building packages, aligning with PEP 517/518. This command ensures that the build process is handled by a build backend specified inpyproject.toml
, making the process more standardized and reliable.- 5-5: Using
twine upload dist/*
for publishing packages is a secure and recommended practice. It ensures that the package is uploaded over HTTPS, protecting the credentials and integrity of the package. This change simplifies and secures the publish process.CHANGELOG.md (1)
- 10-13: The changelog entries clearly document the significant changes made in this PR, including compatibility with SQLAlchemy 2, updates to packaging files per PEP 517 and PEP 518, and the restriction of the Python version to 3.7 or higher. This documentation is essential for users to understand the impact of the changes and adheres to the Keep a Changelog format.
social_flask_sqlalchemy/models.py (2)
- 15-16: The introduction of
PSABase
as a base class for SQLAlchemy models, inheriting fromDeclarativeBase
, is a good practice for organizing model inheritance and ensuring compatibility with SQLAlchemy 2. This change aligns with the PR's objective of updating the codebase for SQLAlchemy 2 compatibility.- 79-80: The adjustment of the
user_id
column declaration inUserSocialAuth
to usemapped_column
with aForeignKey
constraint and setting it as non-nullable and indexed is a correct and efficient way to declare model relationships in SQLAlchemy 2. This change ensures that theUserSocialAuth
model is compatible with SQLAlchemy 2's standards and practices.
for more information, see https://pre-commit.ci
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
pyproject.toml
is excluded by:!**/*.toml
Files selected for processing (2)
- .pre-commit-config.yaml (1 hunks)
- social_flask_sqlalchemy/models.py (4 hunks)
Additional comments: 7
.pre-commit-config.yaml (5)
- 4-10: The pre-commit hook configuration for
pre-commit/pre-commit-hooks
is correctly set up with standard hooks likecheck-merge-conflict
,debug-statements
, andmixed-line-ending
. Themixed-line-ending
hook is specifically configured to fix line endings to LF (--fix=lf
), which is a good practice for cross-platform consistency.- 11-15: The
isort
hook is correctly configured with the--profile=black
argument, ensuring that imports are sorted in a manner that is compatible with theblack
code formatter. This consistency between formatters is crucial for maintaining code quality.- 21-24: The
black
formatter is correctly configured without specific arguments. This is standard practice, asblack
has its own opinionated formatting rules that usually don't require customization.- 25-28: The
flake8
linter is set up without specific arguments. Ensure that the project's coding standards are fully compatible withflake8
's default rules, or consider configuring it with a.flake8
configuration file for more granular control.- 33-37: The
pretty-format-yaml
hook is configured with--autofix
and--indent '2'
, which automatically fixes YAML files to have a 2-space indentation. This is a good practice for maintaining consistency in YAML file formatting across the project.social_flask_sqlalchemy/models.py (2)
- 16-17: Refactoring the
PSABase
class to useDeclarativeBase
aligns with SQLAlchemy 2's recommended practices for declaring base classes. This change ensures that the models are correctly integrated with the SQLAlchemy ORM.- 79-80: The use of
mapped_column
for theuser_id
column inUserSocialAuth
is a significant change that aligns with SQLAlchemy 2's new paradigm. However, ensure that all references toUserSocialAuth.user_id
throughout the codebase are updated to accommodate this change, if necessary.Verification successful
The search for references to
UserSocialAuth.user_id
in the project only found its declaration insocial_flask_sqlalchemy/models.py
. This indicates that there are no other parts of the codebase that directly referenceUserSocialAuth.user_id
, suggesting that the change to usemapped_column
does not require updates elsewhere to accommodate this change. Therefore, the concern mentioned in the review comment does not apply.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for references to UserSocialAuth.user_id in the project rg "UserSocialAuth.user_id"Length of output: 105
- repo: https://github.com/asottile/pyupgrade | ||
rev: v3.15.1 | ||
hooks: | ||
- id: pyupgrade | ||
args: [--py36-plus] |
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.
The pyupgrade
hook is set to use the --py36-plus
argument. Given the PR's objective to ensure compatibility with Python 3.7 and above, consider updating this argument to --py37-plus
to better align with the project's Python version requirements.
- args: [--py36-plus]
+ args: [--py37-plus]
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
- repo: https://github.com/asottile/pyupgrade | |
rev: v3.15.1 | |
hooks: | |
- id: pyupgrade | |
args: [--py36-plus] | |
- repo: https://github.com/asottile/pyupgrade | |
rev: v3.15.1 | |
hooks: | |
- id: pyupgrade | |
args: [--py37-plus] |
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- social_flask_sqlalchemy/init.py (1 hunks)
- social_flask_sqlalchemy/models.py (4 hunks)
Files skipped from review due to trivial changes (1)
- social_flask_sqlalchemy/init.py
Files skipped from review as they are similar to previous changes (1)
- social_flask_sqlalchemy/models.py
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Proposed changes
With the introduction of PEP 517/518, the Python package build process has changed to allow the use of multiple build backends. We've improved the build process accordingly.
In addition, we improved the program to ensure that several previously developed libraries work properly with the newly updated SQLAlchemy 2 in January 2023.
Please consider this PR and look forward to the new version release. I took the time to fix the program and submit the patch because I'm planning to include it in a Korean Flask-based programming book I'm writing.
Types of changes
Please check the type of change your PR introduces:
Checklist
Put an
x
in the boxes that apply. You can also fill these out after creatingthe PR. If you're unsure about any of them, don't hesitate to ask. We're here to
help! This is simply a reminder of what we are going to look for before merging
your code.
Summary by CodeRabbit
.pre-commit-config.yaml
file.__version__
variable insocial_flask_sqlalchemy/__init__.py
.social_flask_sqlalchemy/models.py
.