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

fix(batch-exports): Create async session in async function #25579

Merged
merged 2 commits into from
Oct 15, 2024

Conversation

tomasfarias
Copy link
Contributor

Problem

Creating a client session outside async function could have problems (see here). Let's maintain a connection in the context manager, otherwise error.

Changes

Create connection on async context manager __aenter__.
Close connection on __aexit__.
Raise error if attempting to run query outside context manager.

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Does this work well for both Cloud and self-hosted?

How did you test this code?

@tomasfarias tomasfarias requested review from a team and removed request for a team October 15, 2024 00:00
@tomasfarias tomasfarias marked this pull request as draft October 15, 2024 00:01
@tomasfarias tomasfarias force-pushed the fix/create-session-in-async-func branch from 6374ee6 to 3475f70 Compare October 15, 2024 00:16
@tomasfarias tomasfarias requested a review from a team October 15, 2024 00:20
@tomasfarias tomasfarias marked this pull request as ready for review October 15, 2024 00:20
@tomasfarias tomasfarias force-pushed the fix/create-session-in-async-func branch from 3475f70 to 7dfb3cd Compare October 15, 2024 00:30
@tomasfarias tomasfarias force-pushed the fix/create-session-in-async-func branch from 7dfb3cd to 13069ee Compare October 15, 2024 00:39
@EDsCODE EDsCODE merged commit 5ed491b into master Oct 15, 2024
86 checks passed
@EDsCODE EDsCODE deleted the fix/create-session-in-async-func branch October 15, 2024 01:20
EDsCODE added a commit that referenced this pull request Oct 15, 2024
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