-
Notifications
You must be signed in to change notification settings - Fork 477
genpoi UUID improvements #1267
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
genpoi UUID improvements #1267
Conversation
* When reading the cache, catch some errors on load, instead of crashing * When writing to cache, write to tmp file, then move it into place. This should be more robust if a ctrl+c is recieved while writing the cache Addresses #1266
try: | ||
gz = gzip.GzipFile(cache_file + ".tmp", "wb") | ||
json.dump(cls.uuid_cache, gz) | ||
os.rename(cache_file + ".tmp", cache_file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we use stuff from files.py in here in case rename doesn't work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I concur! There's a whole thing for this IIRC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the library you are referring too? How do you all feel about adding a new 3rd party dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, I'm talking about files.py in overviewer_core... which, uh... I think you wrote.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey neat! i forgot about that! ok yeah, we should use that
About saving the corrupted uuid cache: do we ever actually expect somebody to ungzip the file and fix it? Or fix it so it's un-gzippable? Why not just delete the cache? |
Someone from IRC convinced me that silently deleting files wasn't a good idea. It was a weak argument, and the only plausible explanation I saw was: someone accidentally copied important data into the UUID cache. |
This should be more robust if a ctrl+c is recieved while writing the
cache
Addresses #1266
cc @ion1 @agrif
I'll merge this in a few days, so let me know if you have any comments