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

Deprecated thread local strategy #5140

Closed
greenape opened this issue May 10, 2022 · 2 comments · Fixed by #5228
Closed

Deprecated thread local strategy #5140

greenape opened this issue May 10, 2022 · 2 comments · Fixed by #5228
Labels
FlowMachine Issues related to FlowMachine refactoring

Comments

@greenape
Copy link
Member

We're stuck on an old version of pandas when using flowmachine, because new pandas uses new sqlalchemy which deprecated threadlocal strategy.

We assume threadlocal throughout flowmachine's codebase at present in a way which is quite challenging to unpick, because one of the side effects of it is that one can be in a transaction in a very concealed way. For example, everything in write_query_to_cache is in a single transaction, but this is not immediately obvious.

We're quite inconsistent about when we pass in a connection (and what a connection is when passed), and when we summon one from context using get_db(), which also makes this a bit difficult to resolve.

@greenape
Copy link
Member Author

I'm wondering if the way to resolve this is to never pass the db connection?

The db connection is available from context, what if we extend that and say db accessible only through context handler, there are two handlers: fresh transaction and nested within current transaction?

@greenape
Copy link
Member Author

Although see https://docs.sqlalchemy.org/en/14/core/connections.html#arbitrary-transaction-nesting-as-an-antipattern for a counter to that, suggesting we should be explicitly passing around instead, which tbh is probably better, but more annoying to migrate to given where we're at.

@greenape greenape mentioned this issue May 23, 2022
@greenape greenape mentioned this issue Jun 16, 2022
8 tasks
@mergify mergify bot closed this as completed in #5228 Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FlowMachine Issues related to FlowMachine refactoring
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant