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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sync/async issues with Asgiref and officialize PsycopgConnector as the default one #753

Merged
merged 23 commits into from Jan 13, 2024

Conversation

ewjoachim
Copy link
Member

@ewjoachim ewjoachim commented Mar 4, 2023

Closes #751

See #751 (comment)

Breaking changes:

  • Aiopg and Psycopg2 connectors are now contrib. To use them, you'll need to import them with from procrastinate.contrib.aiopg import AiopgConnector and from procrastinate.contrib.psycopg2 import Psycopg2Connector. Also, the dependencies to aiopg and psycopg2 are now optional: use pip install procrastinate[aiopg] or pip install procrastinate[psycopg2] (or pip install procrastinate[aiopg,psycopg2] for both)
  • The main supported connector is now the PsycopgConnector that uses Psycopg 3 (and SyncPsycopgConnector). Please note that PsycopgConnector accepts parameters based on psycopg_pool.ConnectionPool, which has a slightly different signature from the psycopg2/aiopg counterpart, the main difference is that the connection arguments are now passed as PsycopgConnector(kwargs={"host": "..."}) (instead of AiopgConnector(host=...)). Other parameters may have changed too, please check the documentation.
  • Actually, you probably won't need sync connectors anymore because the Async connectors are now able to derive a sync connector when called in a sync context. You should try defining a single (async) connector in your code such as PsycopgConnector (or AiopgConnector), see if it works with all your existing code and don't hesitate to report potential issues. Synchronous connectors are still available in case you need it, so it should be a workaround in most cases.
  • Synchronous tasks are now launched asynchronously in a ThreadPoolExecutor using asgiref.sync_to_async. That said, because of the Global Interpreter Lock (GIL), CPU-consuming tasks will not run faster with parallelization.
  • The CLI parser has changed. The main differences should be around the order of arguments vs flag, and around environment variables. If you find something unexpected, please open an issue.
  • Opening your app at the same time you instantiate it is now discouraged. Ideally, define your app as a module variable, and open it in the appropriate function (when your process starts). In the future, we may expose new helpers for doing this easily with django. Note that when you use the Procrastinate CLI, it takes care of opening/closing the app properly.
  • JobManager.check_connection was an async method. It became check_connection_async for consistency. check_connection was created a the sync counterpart.

Successful PR Checklist:

  • Tests
    • (not applicable?)
  • Documentation
    • (not applicable?)

Todolist

  • Launch acceptance tests with aiopg
  • Put aiopg & psycopg2 dependencies as extras, update doc
  • Implement get_sync_connector for aiopg (> psycopg2) and psycopg2 (>self)
  • Maybe move aiopg & psycopg2 in contrib, update doc because we won't be able to import them from procrastinate directly. Add wrappers to make the error explicit
  • Test the demo & quickstart extensively, redo demos documentation
  • Investigate all the missing coverage spots & add corresponding tests
  • Draft the exact list of methods that have been deleted for the breaking change message > Looks like no method was deleted :o
  • Isolate asgiref dependency to utils.py
  • Craft acceptable commits 馃槄

Copy link

github-actions bot commented Nov 1, 2023

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
聽聽procrastinate
聽聽__init__.py
聽聽__main__.py
聽聽app.py 264
聽聽cli.py 51
聽聽connector.py
聽聽exceptions.py
聽聽manager.py
聽聽psycopg_connector.py 164-166, 239, 298
聽聽schema.py
聽聽shell.py 41-45
聽聽sync_psycopg_connector.py 33, 151, 176
聽聽testing.py
聽聽utils.py
聽聽worker.py
聽聽procrastinate/contrib/aiopg
聽聽__init__.py
聽聽aiopg_connector.py 207-208
聽聽procrastinate/contrib/psycopg2
聽聽__init__.py
聽聽psycopg2_connector.py
聽聽procrastinate/contrib/sqlalchemy
聽聽psycopg2_connector.py 120
Project Total

This report was generated by python-coverage-comment-action

@ewjoachim ewjoachim marked this pull request as ready for review November 1, 2023 19:06
@ewjoachim ewjoachim requested a review from a team as a code owner November 1, 2023 19:06
@ewjoachim ewjoachim marked this pull request as draft November 7, 2023 14:46
@ewjoachim ewjoachim force-pushed the sync-async branch 6 times, most recently from 2578abd to c983347 Compare January 7, 2024 20:44
@ewjoachim ewjoachim changed the title Add asgiref, update all Fix sync/async issues with Asgiref and officialize PsycopgConnector as the default one Jan 7, 2024
@ewjoachim ewjoachim force-pushed the sync-async branch 5 times, most recently from ea6d605 to c1d848e Compare January 7, 2024 23:34
@ewjoachim ewjoachim marked this pull request as ready for review January 7, 2024 23:35
Also, take into account the new sync/async behaviour
@ewjoachim ewjoachim merged commit a0270b5 into main Jan 13, 2024
8 checks passed
@ewjoachim ewjoachim deleted the sync-async branch January 13, 2024 13:11
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.

Fix the sync/async compatibilty
1 participant