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

Move caching of ConfintStore inside permute_picking. #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

njohner
Copy link

@njohner njohner commented Jul 2, 2024

Caching an SQL connection on import of scoary works fine when used as a command line tool, but not when scoary is integrated in another python application, as this can lead to the SQL object being used in a different thread than the one it was created in.

This led to issues in the webapplication into which I integrated scoary. I've actually fixed it there (see metagenlab/zDB#101), so I do not need this fix here, but I think it might be useful if anyone else wants to integrate scoary in a python application.

To fix this I simply moved the caching into the permute_picking function, so the connection is renewed every time that function is called. I think that should not have any negative impact on performance.

Caching an SQL connection on import of scoary works fine when used
as a command line tool, but not when scoary is integrated in another
python application, as this can lead to the SQL object being used
in a different thread than the one it was created in.
@MrTomRod
Copy link
Owner

MrTomRod commented Jul 2, 2024

Thanks for telling me about the bug and suggesting a fix! I did it slightly differently and pushed my fix to the dev branch:

https://github.com/MrTomRod/scoary-2/compare/dev

Do you agree that my fix also solves the issue, and is slightly more elegant?

@njohner
Copy link
Author

njohner commented Jul 2, 2024

Hi,
Yes looks good. I guess you chose to always connect in the __init__ to check the connection anyway, even when disconnecting right afterwards?
Anyway, that works for me thanks!

@njohner
Copy link
Author

njohner commented Jul 2, 2024

Oh and I just saw the paper is out, congrats. I'll update the reference in zDB...

@MrTomRod
Copy link
Owner

MrTomRod commented Jul 2, 2024

Thanks!

I connected in __init__ to create the tables in the database if they don't exist, and then stayed connected just because it's simpler.

Now, it connects in __init__, creates the database, and disconnects. Whenever the function is run, it connects again does it's thing, and disconnects at the end of the function.

I'll wait before creating v0.0.6, ok? Or is it easier for you if I publish the fix?

@njohner
Copy link
Author

njohner commented Jul 2, 2024

You can wait, no problem, I'm not in a hurry as I fixed it in zDB for now.

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