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 cache workfile collision between versions #1399

Merged
merged 1 commit into from Aug 21, 2014

Conversation

jvdp
Copy link
Contributor

@jvdp jvdp commented Jun 4, 2014

After struggling with this for quite a few hours, it turns out that the changes to the cache from #1312 are not versions aware: during caching, different versions share the same workfile which gets deleted in between. Effectively, this breaks cache use for uploaders with two versions or more.

This patch ensures each version creates its own workfile. There are probably still problems if move_to_cache is defined as true, and perhaps this issue can also be solved by somehow sharing workfiles without deleting until all are done with it... The problem also does not seem to occur if the file storage mechanism is used, which makes it hard to add a failing test (I presume because of the state tracked by the SanitizedFile class, which is reused.) There's a lot of moving parts involved here...

@stephankaag
Copy link
Contributor

This is a major bug. Versioning is broken when using fog as cache-storage and using >1 versions.

Your fix works for me but might require a test before being pulled in.

@jvdp
Copy link
Contributor Author

jvdp commented Jun 8, 2014

Any ideas how, under the fog tests perhaps? Or just a test that checks the workfile path itself?

@jvdp
Copy link
Contributor Author

jvdp commented Jun 10, 2014

I changed the fix slightly (the workfile_path method should probably not live in the Versions module after all) and included a simple test.

@stephankaag
Copy link
Contributor

CC @bensie

@stephankaag
Copy link
Contributor

@bensie What is missing before this can be merged in?

bensie added a commit that referenced this pull request Aug 21, 2014
Fix cache workfile collision between versions
@bensie bensie merged commit be840d1 into carrierwaveuploader:master Aug 21, 2014
@bensie
Copy link
Member

bensie commented Aug 21, 2014

Merged...looks good!

@mshibuya
Copy link
Member

@jvdp Just got exactly same issue and your fix saved lots of my time, thanks!

@jvdp
Copy link
Contributor Author

jvdp commented Aug 28, 2014

@mshibuya great to hear that it works :)

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.

None yet

4 participants