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

Replace synchronous database driver with async #1131

Draft
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

spwoodcock
Copy link
Member

@spwoodcock spwoodcock commented Jan 23, 2024

What type of PR is this? (check all applicable)

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation Update
  • 🎨 Style
  • πŸ§‘β€πŸ’» Code Refactor
  • πŸ”₯ Performance Improvements
  • βœ… Test
  • πŸ€– Build
  • πŸ” CI
  • πŸ“¦ Chore (Release)
  • ⏩ Revert

Related Issue

Related to #254

Describe this PR

WORK IN PROGRESS

We should probably do this alongside #254 to avoid having to implement in SQLAlchemy
(the structure remains mostly the same when using the underlying driver)

Either (if using SQLAlchemy):

  • Replace all db.query code with db.select or similar.

OR (if using psycopg/asyncpg):

  • Replace all db.query code with direct driver usage via SQL.

Also:

  • Add await to every instance of db usage...
  • Fix running of test cases (untested).

Useful reference: https://praciano.com.br/fastapi-and-async-sqlalchemy-20-with-pytest-done-right.html

Checklist before requesting a review

[optional] What gif best describes this PR or how it makes you feel?

@robsavoye
Copy link
Collaborator

2 similar comments
@robsavoye
Copy link
Collaborator

@robsavoye
Copy link
Collaborator

@robsavoye
Copy link
Collaborator

There is a new database driver in osm-rawdata that is fuylly async. Just replace tm_admin.postgres with tm_admin.pgsync. I wrote a brief doc on how to use this module. https://github.com/hotosm/tm-admin/blob/main/docs/pgasync.md.

@spwoodcock
Copy link
Member Author

From my understanding tm-admin won't replace the entire backend, but mostly the logic around administration of projects, users, orgs etc.

This PR is using psycopg3, but tm-admin uses asyncpg. To avoid additional dependencies, once tm-admin is integrated this PR should probably be updated to use asyncpg.

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

Successfully merging this pull request may close these issues.

None yet

2 participants