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

Monkeypatch Whoosh to use mkdtemp rather than a fixed temporary directory #2310

Merged
merged 1 commit into from May 5, 2016

Conversation

Projects
None yet
3 participants
@natefoo
Copy link
Member

commented May 4, 2016

Whoosh's RamStorage creates a MAIN.tmp directory in the default temporary directory to do some work in during indexing. This is not safe in a multiprocess environment because one process will remove the directory while others are using it. Although apparently this is not a problem in and of itself since the author merged a pull request that catches the subsequent exception that is caused by later processes attempting to remove the directory after it's already been done by the first one.

Regardless, that change is in an as-yet-unreleased version of Whoosh, and honestly I don't understand why you would want to use a fixed directory name in the first place. This PR monkeypatches Whoosh to just use mkdtemp() instead.

xref: https://bitbucket.org/mchaput/whoosh/issues/391/race-conditions-with-temp-storage

@nsoranzo

This comment has been minimized.

Copy link
Member

commented May 5, 2016

Any chance of (also) fix this upstream with a PR?

@martenson

This comment has been minimized.

Copy link
Member

commented May 5, 2016

@nsoranzo I commented on the Whoosh issue with a link to this solution. Given that the author already merged the alternative (catching and ignoring notfound exceptions) I am not sure it is worth to dig in it. Feel free to, though.

@martenson martenson merged commit 432ec84 into galaxyproject:dev May 5, 2016

4 checks passed

api test Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished.
Details
toolshed test Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.