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

Data recovery #24

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Data recovery #24

wants to merge 3 commits into from

Conversation

egorps
Copy link

@egorps egorps commented Dec 10, 2013

Send data to a file when an error occurs.

it simply discards the data and removes the file.
"""
self.recover_file = os.path.join(tempfile.mkdtemp(), 'saved_inserts')
file = open(self.recover_file, 'w')
Copy link
Contributor

Choose a reason for hiding this comment

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

make sure file name is unique across runs by suffixing it with pid + timestamp

suffix = '.' + str(os.getpid()) + '.' + datetime.datetime.utcnow().strftime('%Y%m%d_%H%M%S')
file = tempfile.NamedTemporaryFile(dir=NeedToGetFromParam, mode='w', delete=false, suffix=suffix)
self.recover_file = file.name

@lomignet
Copy link
Contributor

Hi John,

This is a great feature, there are only 2 things on which I would like your thoughts:

  1. I commented a bit on your code. Basically, I would like to see the top dir of the drained file to be configurable. On our setup (not exceptional I believe), /tmp is a small memory tmpfs. This is thus non persistent, and could be filled quite quickly. My (maybe dirty) idea would be to make drain_to_file a str instead of a boolean, this str would be the root dir of the drainfile. None would have the meaning of the current False.

  2. According to the doc, using mkdtemp, "The directory is readable, writable, and searchable only by the creating user ID.". It feels safer for forensics to use NamedTemporaryFile "the file is guaranteed to have a visible name in the file system". This has the added benefit of allowing mutiple files for multiple runs to be saved, as I added on the comments on my code.

Thanks a lot for your PR.

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