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

Reset corrupted PStore (fixes #409) #418

Merged
merged 2 commits into from Apr 14, 2014

Conversation

denisdefreyne
Copy link
Member

This PR makes a store reset itself when corruption is detected. This is fine, because the stores only contain data to speed up site compilation.

Fix for #409.

@denisdefreyne
Copy link
Member Author

CC @bobthecow

@denisdefreyne denisdefreyne changed the title Reset corrupted PStore Reset corrupted PStore (fixes #409) Apr 13, 2014
@loaded = true
rescue
FileUtils.rm_f(filename)
load
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't like an this rescue, since we're calling #load again inside the handler. anything that fails which isn't fixable by removing the pstore will just go into an infinite loop. Can we rescue a specific error?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

We can rescue TypeError, but to be 100% safe, the #load method should probably take an argument that indicates whether or not this invocation is a retry, and if it is, do not retry but just reraise the error. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep. that works too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, it’s actually not easy at all to make this fail (I noticed while trying to write a test for this). The only thing where this double-call check is necessary is when FileUtils.rm_f(filename) fails, because in the next call, the function will exit before the begin/rescue/end block, because !File.file?(filename).

So I think it’s OK to stick with the original implementation.

denisdefreyne added a commit that referenced this pull request Apr 14, 2014
@denisdefreyne denisdefreyne merged commit afc8434 into release-3.6.x Apr 14, 2014
@denisdefreyne denisdefreyne deleted the fix/reset-broken-pstore branch April 14, 2014 07:13
@denisdefreyne
Copy link
Member Author

Merged anyway, but you can always give more feedback later :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants