search

conda-incubator / conda-store

Data science environments, for collaboration. ✨
https://conda.store
BSD 3-Clause "New" or "Revised" License
143 stars 46 forks source link

Bug fix: use context manager for Sessions #622

Closed nkaretnikov closed 11 months ago

nkaretnikov commented 11 months ago

Fixes #598.

netlify[bot] commented 11 months ago

Deploy Preview for kaleidoscopic-dango-0cf31d canceled.

Name Link
Latest commit 9053e863d8d9f39e2cbcf19a86c76a3ead7c38bf
Latest deploy log https://app.netlify.com/sites/kaleidoscopic-dango-0cf31d/deploys/652bd77bdbd176000801a7c6
nkaretnikov commented 11 months ago

Note: I deliberately open this new context manager at the start of each request to avoid any unintended side-effects related to DB object lifetimes. This also make it easier to review since there are no logic changes. Later (and with more tests), this can be scoped better -- only when db is used.

I also make this context manager a method of CondaStore, to avoid refactoring of session_factory, like introducing globals.

Also: this diff will be more readable if you hide whitespace changes in the GitHub UI:

Screen Shot 2023-10-15 at 13 44 03
nkaretnikov commented 11 months ago

@costrouc PTAL.

iameskild commented 11 months ago

Amazing work @nkaretnikov! 🎉