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

interoperability with asyncio (part 1) #174

Merged
merged 5 commits into from
Jun 5, 2024
Merged

interoperability with asyncio (part 1) #174

merged 5 commits into from
Jun 5, 2024

Conversation

achille-roussel
Copy link
Contributor

@achille-roussel achille-roussel commented Jun 4, 2024

This PR modifies the dispatch-py internals to build on the asyncio package in order to better integrate with frameworks like FastAPI, httpx, etc... and any other Python code that uses async/await constructs for I/O operations.

A couple of notable changes:

  • The signature of primitive functions is changed to always be async, this is the only breaking change
  • Due to interacting with the asyncio event loop, there are changes to the order of some operations. Notably, the depth-first traversal does not follow the same order anymore; this shouldn't have any impact on production applications since they wouldn't (shouldn't) rely on the order of async operations

In a few places, we are constructing temporary event loops (using a class similar to asyncio.Runner) to convert from blocking to asyncio code, for example in the http server we use in dispatch.run(). This a bit hacky and probably not optimal, we could do better by taking a dependency on aiohttp, I'll look into it in a follow up PR where I also want to investigate how we could integration the .dispatch() method with asyncio.

Fixes #122

Signed-off-by: Achille Roussel <achille.roussel@gmail.com>
Signed-off-by: Achille Roussel <achille.roussel@gmail.com>
@achille-roussel achille-roussel added enhancement New feature or request breaking change Breaking change that will be disruptive to programs that depended on them labels Jun 4, 2024
@achille-roussel achille-roussel self-assigned this Jun 4, 2024
Signed-off-by: Achille Roussel <achille.roussel@gmail.com>
Signed-off-by: Achille Roussel <achille.roussel@gmail.com>
Signed-off-by: Achille Roussel <achille.roussel@gmail.com>
Copy link
Contributor

@chriso chriso left a comment

Choose a reason for hiding this comment

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

👏🏻

Comment on lines 132 to +134
self.assert_poll_call_functions(
output,
["a", "b", "c", "d", "e", "f", "g", "h"],
["d", "h", "e", "f", "g", "a", "b", "c"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we update this assertion helper to sort both lists before comparison? Then we don't have to adapt the test if the order changes across Python versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered it but I thought it would be better to keep it simple and be notified if the order ever changes, sorting could mask other unintended changes, let's keep this for when we know we have a problem, so far the order is the same from 3.8 to 3.12.

@achille-roussel achille-roussel merged commit ffc358b into main Jun 5, 2024
7 checks passed
@achille-roussel achille-roussel deleted the asyncio branch June 5, 2024 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Breaking change that will be disruptive to programs that depended on them enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Interoperability with asyncio
3 participants