Merge lp:~frankban/juju-quickstart/envs-backup into lp:juju-quickstart

Proposed by Francesco Banconi
Status: Merged
Merged at revision: 45
Proposed branch: lp:~frankban/juju-quickstart/envs-backup
Merge into: lp:juju-quickstart
Diff against target: 244 lines (+114/-9)
6 files modified
quickstart/manage.py (+4/-1)
quickstart/models/envs.py (+18/-4)
quickstart/tests/models/test_envs.py (+45/-0)
quickstart/tests/test_manage.py (+13/-4)
quickstart/tests/test_utils.py (+19/-0)
quickstart/utils.py (+15/-0)
To merge this branch: bzr merge lp:~frankban/juju-quickstart/envs-backup
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+201449@code.launchpad.net

Description of the change

Create a backup copy of the envs file.

In a pre-imp call with Gary, we decided the
following strategy: a backup copy of the
environments.yaml file is created once per
quickstart session the first time environments
are changed by the user. This way we prevent the
original backup to be overwritten if the user
makes two subsequent changes to the envs.

Tests: `make check`.

QA:

- Create a backup copy of your environments file:
  `cp ~/.juju/environments.yaml ~/environments.yaml.bak`.

- Run `.venv/bin/python juju-quickstart -i`, select one
  of your environments, exit with ^X without changing anything.
  Check your original file has not been changed.

- Run `.venv/bin/python juju-quickstart -i`, change something
  (e.g. create, remove or change an environment, or just change
  the default one). Change at least two environments.
  Exit with ^X. The envs.yaml file now should be the one
  generated by quickstart and should reflect your changes.
  It also includes the path to the backup copy, which
  should be your original file.

- Restore your original environments file:
  `mv ~/environments.yaml.bak ~/.juju/environments.yaml`.

Done, thank you!

https://codereview.appspot.com/51520043/

To post a comment you must log in.
Revision history for this message
Francesco Banconi (frankban) wrote :

Reviewers: mp+201449_code.launchpad.net,

Message:
Please take a look.

Description:
Create a backup copy of the envs file.

In a pre-imp call with Gary, we decided the
following strategy: a backup copy of the
environments.yaml file is created once per
quickstart session the first time environments
are changed by the user. This way we prevent the
original backup to be overwritten if the user
makes two subsequent changes to the envs.

Tests: `make check`.

QA:

- Create a backup copy of your environments file:
   `cp ~/.juju/environments.yaml ~/environments.yaml.bak`.

- Run `.venv/bin/python juju-quickstart -i`, select one
   of your environments, exit with ^X without changing anything.
   Check your original file has not been changed.

- Run `.venv/bin/python juju-quickstart -i`, change something
   (e.g. create, remove or change an environment, or just change
   the default one). Change at least two environments.
   Exit with ^X. The envs.yaml file now should be the one
   generated by quickstart and should reflect your changes.
   It also includes the path to the backup copy, which
   should be your original file.

- Restore your original environments file:
   `mv ~/environments.yaml.bak ~/.juju/environments.yaml`.

Done, thank you!

https://code.launchpad.net/~frankban/juju-quickstart/envs-backup/+merge/201449

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/51520043/

Affected files (+116, -9 lines):
   A [revision details]
   M quickstart/manage.py
   M quickstart/models/envs.py
   M quickstart/tests/models/test_envs.py
   M quickstart/tests/test_manage.py
   M quickstart/tests/test_utils.py
   M quickstart/utils.py

61. By Francesco Banconi

Fix typo.

Revision history for this message
Francesco Banconi (frankban) wrote :

Matthew approved this MP, but he's not currently able to log in to
Rietveld.
Thanks for the review, submitting now.

https://codereview.appspot.com/51520043/

Revision history for this message
Francesco Banconi (frankban) wrote :

*** Submitted:

Create a backup copy of the envs file.

In a pre-imp call with Gary, we decided the
following strategy: a backup copy of the
environments.yaml file is created once per
quickstart session the first time environments
are changed by the user. This way we prevent the
original backup to be overwritten if the user
makes two subsequent changes to the envs.

Tests: `make check`.

QA:

- Create a backup copy of your environments file:
   `cp ~/.juju/environments.yaml ~/environments.yaml.bak`.

- Run `.venv/bin/python juju-quickstart -i`, select one
   of your environments, exit with ^X without changing anything.
   Check your original file has not been changed.

- Run `.venv/bin/python juju-quickstart -i`, change something
   (e.g. create, remove or change an environment, or just change
   the default one). Change at least two environments.
   Exit with ^X. The envs.yaml file now should be the one
   generated by quickstart and should reflect your changes.
   It also includes the path to the backup copy, which
   should be your original file.

- Restore your original environments file:
   `mv ~/environments.yaml.bak ~/.juju/environments.yaml`.

Done, thank you!

R=
CC=
https://codereview.appspot.com/51520043

https://codereview.appspot.com/51520043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'quickstart/manage.py'
--- quickstart/manage.py 2014-01-13 15:30:44 +0000
+++ quickstart/manage.py 2014-01-13 16:51:49 +0000
@@ -25,6 +25,7 @@
25import codecs25import codecs
26import logging26import logging
27import os27import os
28import shutil
28import sys29import sys
29import webbrowser30import webbrowser
3031
@@ -155,9 +156,11 @@
155 application with an error if an OSError exception is raised while saving156 application with an error if an OSError exception is raised while saving
156 the environments database.157 the environments database.
157 """158 """
159 backup_function = utils.run_once(shutil.copyfile)
160
158 def save_callable(env_db):161 def save_callable(env_db):
159 try:162 try:
160 envs.save(env_file, env_db)163 envs.save(env_file, env_db, backup_function=backup_function)
161 except OSError as err:164 except OSError as err:
162 return parser.error(bytes(err))165 return parser.error(bytes(err))
163166
164167
=== modified file 'quickstart/models/envs.py'
--- quickstart/models/envs.py 2014-01-10 09:08:06 +0000
+++ quickstart/models/envs.py 2014-01-13 16:51:49 +0000
@@ -190,24 +190,38 @@
190 return env_db190 return env_db
191191
192192
193def save(env_file, env_db):193def save(env_file, env_db, backup_function=None):
194 """Save the given env_db to the provided environments.yaml file.194 """Save the given env_db to the provided environments.yaml file.
195195
196 The new environments are saved to disk in the most atomic way possible.196 The new environments are saved to disk in the most atomic way possible.
197 If backup_function is not None, it is called passing the original
198 environments file path and the backup destination path. The backup is
199 obviously performed only if the file exists.
197200
198 Raise an OSError if any errors occur in the process.201 Raise an OSError if any errors occur in the process.
199 """202 """
203 dirname = os.path.dirname(env_file)
204 backup_file = None
205 if os.path.exists(env_file):
206 if backup_function is not None:
207 # Create a backup of the original file.
208 backup_file = env_file + '.quickstart.bak'
209 backup_function(env_file, backup_file)
210 else:
211 utils.mkdir(dirname)
212 banner = utils.get_quickstart_banner()
200 # Save the contents in a temporary file, then rename to the real file.213 # Save the contents in a temporary file, then rename to the real file.
201 # Since the renaming operation may fail on some Unix flavors if the source214 # Since the renaming operation may fail on some Unix flavors if the source
202 # and destination files are on different file systems, use for the215 # and destination files are on different file systems, use for the
203 # temporary file the same directory where the env_file is stored.216 # temporary file the same directory where the env_file is stored.
204 dirname = os.path.dirname(env_file)
205 utils.mkdir(dirname)
206 banner = utils.get_quickstart_banner()
207 try:217 try:
208 temp_file = tempfile.NamedTemporaryFile(218 temp_file = tempfile.NamedTemporaryFile(
209 prefix='quickstart-temp-envs-', dir=dirname, delete=False)219 prefix='quickstart-temp-envs-', dir=dirname, delete=False)
210 temp_file.write(banner.encode('utf-8'))220 temp_file.write(banner.encode('utf-8'))
221 if backup_file is not None:
222 temp_file.write(
223 '# A backup copy of this file can be found in\n'
224 '# {}\n\n'.format(backup_file))
211 serializers.yaml_dump(env_db, temp_file)225 serializers.yaml_dump(env_db, temp_file)
212 except Exception as err:226 except Exception as err:
213 raise OSError(err)227 raise OSError(err)
214228
=== modified file 'quickstart/tests/models/test_envs.py'
--- quickstart/tests/models/test_envs.py 2014-01-09 12:48:20 +0000
+++ quickstart/tests/models/test_envs.py 2014-01-13 16:51:49 +0000
@@ -22,6 +22,8 @@
22import copy22import copy
23import functools23import functools
24import os24import os
25import shutil
26import tempfile
25import unittest27import unittest
2628
27import mock29import mock
@@ -199,6 +201,18 @@
199 self.env_file = self.make_env_file()201 self.env_file = self.make_env_file()
200 self.original_contents = open(self.env_file).read()202 self.original_contents = open(self.env_file).read()
201203
204 def make_juju_home(self):
205 """Create a temporary Juju home directory.
206
207 Return the Juju home path and the path of the corresponding
208 environments file.
209 """
210 playground = tempfile.mkdtemp()
211 self.addCleanup(shutil.rmtree, playground)
212 juju_home = os.path.join(playground, '.juju')
213 os.mkdir(juju_home)
214 return juju_home, os.path.join(juju_home, 'environments.yaml')
215
202 def test_dump_error(self):216 def test_dump_error(self):
203 # An OSError is raised if errors occur in the serialization process.217 # An OSError is raised if errors occur in the serialization process.
204 with mock.patch('quickstart.serializers.yaml_dump') as mock_yaml_dump:218 with mock.patch('quickstart.serializers.yaml_dump') as mock_yaml_dump:
@@ -238,6 +252,37 @@
238 # Also the new contents have been saved.252 # Also the new contents have been saved.
239 self.assertIn('new: contents', contents)253 self.assertIn('new: contents', contents)
240254
255 def test_backup(self):
256 # A backup function, if provided, is used to create a backup copy
257 # of the original environments file.
258 mock_backup = mock.Mock()
259 envs.save(
260 self.env_file, {'new': 'contents'}, backup_function=mock_backup)
261 contents = open(self.env_file).read()
262 expected_backup_path = self.env_file + '.quickstart.bak'
263 # The backup function has been called.
264 mock_backup.assert_called_once_with(
265 self.env_file, expected_backup_path)
266 # The new file indicates where to find the backup copy.
267 self.assertIn(
268 '# A backup copy of this file can be found in\n'
269 '# {}\n'.format(expected_backup_path),
270 contents)
271
272 def test_missing_juju_home(self):
273 # The environments file directory is created if not existing.
274 juju_home, env_file = self.make_juju_home()
275 shutil.rmtree(juju_home)
276 envs.save(env_file, {'new': 'contents'})
277 self.assertTrue(os.path.isdir(juju_home))
278
279 def test_backup_missing_juju_home(self):
280 # The backup function is not executed if there is nothing to backup.
281 juju_home, env_file = self.make_juju_home()
282 mock_backup = mock.Mock()
283 envs.save(env_file, {'new': 'contents'}, backup_function=mock_backup)
284 self.assertFalse(mock_backup.called)
285
241286
242class EnvDataTestsMixin(object):287class EnvDataTestsMixin(object):
243 """Set up an initial environments dictionary."""288 """Set up an initial environments dictionary."""
244289
=== modified file 'quickstart/tests/test_manage.py'
--- quickstart/tests/test_manage.py 2014-01-13 15:30:44 +0000
+++ quickstart/tests/test_manage.py 2014-01-13 16:51:49 +0000
@@ -278,13 +278,16 @@
278 self.parser = mock.Mock()278 self.parser = mock.Mock()
279 self.env_file = '/tmp/envfile.yaml'279 self.env_file = '/tmp/envfile.yaml'
280 self.env_db = helpers.make_env_db()280 self.env_db = helpers.make_env_db()
281 self.save_callable = manage._create_save_callable(281 with mock.patch('quickstart.manage.utils.run_once') as mock_run_once:
282 self.parser, self.env_file)282 self.save_callable = manage._create_save_callable(
283 self.parser, self.env_file)
284 self.mock_run_once = mock_run_once
283285
284 def test_saved(self, mock_save):286 def test_saved(self, mock_save):
285 # The returned function correctly saves the new environments database.287 # The returned function correctly saves the new environments database.
286 self.save_callable(self.env_db)288 self.save_callable(self.env_db)
287 mock_save.assert_called_once_with(self.env_file, self.env_db)289 mock_save.assert_called_once_with(
290 self.env_file, self.env_db, backup_function=self.mock_run_once())
288 self.assertFalse(self.parser.error.called)291 self.assertFalse(self.parser.error.called)
289292
290 def test_error(self, mock_save):293 def test_error(self, mock_save):
@@ -292,9 +295,15 @@
292 # occurs while saving the new environments database.295 # occurs while saving the new environments database.
293 mock_save.side_effect = OSError(b'bad wolf')296 mock_save.side_effect = OSError(b'bad wolf')
294 self.save_callable(self.env_db)297 self.save_callable(self.env_db)
295 mock_save.assert_called_once_with(self.env_file, self.env_db)298 mock_save.assert_called_once_with(
299 self.env_file, self.env_db, backup_function=self.mock_run_once())
296 self.parser.error.assert_called_once_with('bad wolf')300 self.parser.error.assert_called_once_with('bad wolf')
297301
302 def test_backup_function(self, mock_save):
303 # The backup function is correctly created.
304 self.save_callable(self.env_db)
305 self.mock_run_once.assert_called_once_with(shutil.copyfile)
306
298307
299class TestStartInteractiveSession(308class TestStartInteractiveSession(
300 helpers.EnvFileTestsMixin, unittest.TestCase):309 helpers.EnvFileTestsMixin, unittest.TestCase):
301310
=== modified file 'quickstart/tests/test_utils.py'
--- quickstart/tests/test_utils.py 2014-01-09 12:31:30 +0000
+++ quickstart/tests/test_utils.py 2014-01-13 16:51:49 +0000
@@ -601,6 +601,25 @@
601 self.assertEqual('zydeco', bsn_series)601 self.assertEqual('zydeco', bsn_series)
602602
603603
604class TestRunOnce(unittest.TestCase):
605
606 def setUp(self):
607 self.results = []
608 self.func = utils.run_once(self.results.append)
609
610 def test_runs_once(self):
611 # The wrapped function runs only the first time it is invoked.
612 self.func(1)
613 self.assertEqual([1], self.results)
614 self.func(2)
615 self.assertEqual([1], self.results)
616
617 def test_wrapped(self):
618 # The wrapped function looks like the original one.
619 self.assertEqual('append', self.func.__name__)
620 self.assertEqual(list.append.__doc__, self.func.__doc__)
621
622
604class TestUnitChanges(unittest.TestCase):623class TestUnitChanges(unittest.TestCase):
605624
606 unit = 'django/42'625 unit = 'django/42'
607626
=== modified file 'quickstart/utils.py'
--- quickstart/utils.py 2014-01-09 11:22:54 +0000
+++ quickstart/utils.py 2014-01-13 16:51:49 +0000
@@ -24,6 +24,7 @@
24import collections24import collections
25import datetime25import datetime
26import errno26import errno
27import functools
27import httplib28import httplib
28import json29import json
29import logging30import logging
@@ -326,6 +327,20 @@
326 return parse_status_output(output, ['machines', '0', 'series'])327 return parse_status_output(output, ['machines', '0', 'series'])
327328
328329
330def run_once(function):
331 """Return a decorated version of the given function which runs only once.
332
333 Subsequent runs are just ignored and return None.
334 """
335 @functools.wraps(function)
336 def decorated(*args, **kwargs):
337 if not decorated.called:
338 decorated.called = True
339 return function(*args, **kwargs)
340 decorated.called = False
341 return decorated
342
343
329def unit_changes(unit_name, changeset):344def unit_changes(unit_name, changeset):
330 """Parse the changeset and return the change related to the given unit.345 """Parse the changeset and return the change related to the given unit.
331346

Subscribers

People subscribed via source and target branches