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 connection pool #210

Merged
merged 5 commits into from
Jul 18, 2023
Merged

Fix connection pool #210

merged 5 commits into from
Jul 18, 2023

Conversation

stefan-jansen
Copy link
Owner

Addresses #209.

@codecov
Copy link

codecov bot commented Jul 18, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +14.69 🎉

Comparison is base (206888c) 73.43% compared to head (03bd61e) 88.12%.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #210       +/-   ##
===========================================
+ Coverage   73.43%   88.12%   +14.69%     
===========================================
  Files         171      171               
  Lines       16057    16058        +1     
===========================================
+ Hits        11791    14151     +2360     
+ Misses       4266     1907     -2359     
Impacted Files Coverage Δ
src/zipline/utils/sqlite_utils.py 100.00% <100.00%> (+4.16%) ⬆️

... and 75 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@stefan-jansen
Copy link
Owner Author

stefan-jansen commented Jul 18, 2023

@MBounouar - adding the fix proposed by @RichardDale; just let me know if you have a better alternative in mind. Looks like something our tests missed.

@RiseT
Copy link

RiseT commented Jul 18, 2023

@MBounouar (sorry, just fixing alias typo 😉 )

@MBounouar
Copy link

@MBounouar - adding the fix proposed by @RichardDale; just let me know if you have a better alternative in mind. Looks like something our tests missed.
Unfortunately, I don't have a better alternative. I vaguely remember about this issue when playing with SQLAlchemy 2.0 several months ago, but I completely forgot about it and never encountered any issue with Linux or WSL.

The whole sql reading and writing in zipline is a bit awkward, even more so with SQLAlchemy 2.0. Refactoring the code or at least investigating potential improvements would make sense. I have the impression that the whole ingestion process is suspiciously slow. However, I don't think I will be able to pick that up any time soon

@stefan-jansen stefan-jansen merged commit 5d1aa4e into main Jul 18, 2023
12 checks passed
@stefan-jansen stefan-jansen deleted the fix_connection_pool branch July 18, 2023 22:24
@RichardDale
Copy link

Thanks Stefan and Mehdi - looking forward to this being pushed to conda-forge so our users can upgrade.

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.

4 participants