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

[Enhancement] Multiple snapshots as new mounts (clone volume) #9 #11

Merged
merged 3 commits into from
Dec 21, 2017

Conversation

marcelo-ochoa
Copy link

Here propesed patch as discussed in Issue #9

@petrus-v
Copy link

Hi,

Thanks for this PR, code looks good to me, can you add a unit test like test_restore about this new functionality and I will be happy to merge it!

@marcelo-ochoa
Copy link
Author

Sure, I'll try to do it, remember I am not a phyton developer ;)
Marcelo.

@petrus-v
Copy link

@marcelo-ochoa,

Yes I remember... but, but with this PR you are proving that now you are a python developer! ;-p

Thanks for that works and take time to invest on it!

@marcelo-ochoa
Copy link
Author

Done test_clone.

test.py Outdated

# modify the file in name2 (new volume)
with open(join(path2, 'foobar'), 'w') as f:
f.write('modified2 foobar')

Choose a reason for hiding this comment

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

Before updating the file in the freshly cloned volume could you make sure it contains foobar to make sure we clone properly create a snapshot?
by adding following snippet before this part:

+        # check the cloned volume has a copy of the original content
+        with open(join(path2, 'foobar')) as f:
+            self.assertEqual(f.read(), 'foobar')

I don't remember if we snapshot twice in an existing volume, but this is an other story related to the snapshot method don't worry about it here!

@petrus-v
Copy link

LGTM.

Thanks!

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

2 participants