Skip to content
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

[proofing] Add pagination for revision list #485

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

akprasad
Copy link
Contributor

@akprasad akprasad commented Apr 1, 2023

The major change here is that we've migrated to flask-sqlalchemy.

flask-sqllachemy is standard in the Flask ecosystem and has useful features like pagination. In addition, useful extensions like flask-debugtoolbar expect it. However, flask-sqlalchemy must have access to the Flask application context and cannot be used outside of it, which means that a migration is "viral" -- if we migrate one part of the code to flask-sqlalchemy, it's worthwhile to migrate everything else as well.

So, this PR also migrates all of our code to use flask-sqlalchemy. By doing so, we can clean up quite a bit of custom code related to SQLAlchemy session management, and we can also standardize our codebase to be consistent with other Flask applications and extensions.

image

@akprasad akprasad requested a review from kvchitrapu April 1, 2023 03:46
ambuda/checks.py Outdated Show resolved Hide resolved
ambuda/models/auth.py Show resolved Hide resolved
ambuda/tasks/__init__.py Show resolved Hide resolved
ambuda/views/proofing/main.py Show resolved Hide resolved
cli.py Outdated Show resolved Hide resolved
cli.py Show resolved Hide resolved
@kvchitrapu
Copy link
Collaborator

kvchitrapu commented Apr 1, 2023

Looks good. Thanks for updating the packages and moving to flask-sqlalchemy. We can paginate on search and replace pages as well.

Copy link
Contributor Author

@akprasad akprasad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review!

ambuda/checks.py Outdated Show resolved Hide resolved
ambuda/models/auth.py Show resolved Hide resolved
ambuda/tasks/__init__.py Show resolved Hide resolved
@kvchitrapu
Copy link
Collaborator

kvchitrapu commented Apr 2, 2023

LGTM. Ship it after resolving conflicts and merging in #486.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants