snapshots: Refactoring and text changes

Signed-off-by: Joseph Nuthalapati <njoseph@thoughtworks.com>

actions/snapshot

@@ -76,10 +76,12 @@ def subcommand_setup(_):
 
 def _set_default_config():
     # Software snapshots' default minimum age: 30 days
-    command = ['snapper','set-config', 'TIMELINE_CREATE=yes',
+    command = [
+        'snapper', 'set-config', 'TIMELINE_CREATE=yes',
         'TIMELINE_LIMIT_HOURLY=10', 'TIMELINE_LIMIT_MONTHLY=2',
         'TIMELINE_LIMIT_WEEKLY=2', 'TIMELINE_LIMIT_YEARLY=0',
-               'TIMELINE_LIMIT_DAILY=3', 'NUMBER_MIN_AGE=2592000']
+        'TIMELINE_LIMIT_DAILY=3', 'NUMBER_MIN_AGE=1296000'
+    ]
     subprocess.run(command, check=True)
 
 

plinth/modules/snapshot/__init__.py

@@ -35,11 +35,10 @@ description = [
     _('Snapshots allows creating and managing filesystem snapshots. These can '
       'be used to roll back the system to a previously known good state in '
       'case of unwanted changes to the system.'),
-    _('Automatic snapshots are taken every hour, day, month and year. Older '
+    _('Automatic snapshots are taken every hour, day, month and year. '
-      'snapshots are automatically deleted keeping 10 of each kind and 50 in '
+      'Although snapshots are efficient and only store the '
-      'total. Although snapshots are efficient and only store the '
       'differences, they may be deleted to reclaim free space.  Individual '
-      'files from older snapshots can be accessed by visiting ".snapshots" '
+      'files from older snapshots can be accessed by visiting "/.snapshots" '
       'directory in the filesystem. Snapshots are not a replacement for '
       'backups.')
 ]
@@ -62,11 +61,12 @@ def setup(helper, old_version=None):
 def get_configuration():
     output = actions.superuser_run('snapshot', ['get-config'])
     output = json.loads(output)
-    return {'enable_timeline_snapshots': output['TIMELINE_CREATE'] == 'yes',
+    return {
+        'enable_timeline_snapshots': output['TIMELINE_CREATE'] == 'yes',
         'hourly_limit': output['TIMELINE_LIMIT_HOURLY'],
         'daily_limit': output['TIMELINE_LIMIT_DAILY'],
         'weekly_limit': output['TIMELINE_LIMIT_WEEKLY'],
         'yearly_limit': output['TIMELINE_LIMIT_YEARLY'],
         'monthly_limit': output['TIMELINE_LIMIT_MONTHLY'],
         'number_min_age': round(int(output['NUMBER_MIN_AGE']) / 86400),
-            'timeline_min_age': round(int(output['TIMELINE_MIN_AGE']) / 86400)}
+    }

plinth/modules/snapshot/forms.py

@@ -27,9 +27,30 @@ class SnapshotForm(forms.Form):
         label=_('Enable Timeline Snapshots'), required=False, help_text=_(
             'Uncheck this to disable timeline snapshots '
             '(hourly, daily, monthly and yearly).'))
-    hourly_limit = forms.IntegerField(label=_('Hourly Snapshots Limit'),required=False, min_value=0)
+
-    daily_limit = forms.IntegerField(label=_('Daily Snapshots Limit'),required=False, min_value=0)
+    hourly_limit = forms.IntegerField(
-    weekly_limit = forms.IntegerField(label=_('Weekly Snapshots Limit'),required=False, min_value=0)
+        label=_('Hourly Snapshots Limit'), min_value=0,
-    monthly_limit = forms.IntegerField(label=_('Monthly Snapshots Limit'),required=False, min_value=0)
+        help_text=('Snapper will only keep this number of hourly snapshots.'))
-    yearly_limit = forms.IntegerField(label=_('Yearly Snapshots Limit'),required=False, min_value=0)
+
-    number_min_age = forms.IntegerField(label=_('Software Snapshots Minimum Age (days)'), required=False, min_value=0)
+    daily_limit = forms.IntegerField(
+        label=_('Daily Snapshots Limit'), min_value=0,
+        help_text=('Snapper will only keep this number of daily snapshots.'))
+
+    weekly_limit = forms.IntegerField(
+        label=_('Weekly Snapshots Limit'), min_value=0,
+        help_text=('Snapper will only keep this number of weekly snapshots.'))
+
+    monthly_limit = forms.IntegerField(
+        label=_('Monthly Snapshots Limit'), min_value=0,
+        help_text=('Snapper will only keep this number of monthly snapshots.'))
+
+    yearly_limit = forms.IntegerField(
+        label=_('Yearly Snapshots Limit'), min_value=0,
+        help_text=('Snapper will only keep this number of yearly snapshots. '
+                   'The default is 0 (disabled).'))
+
+    number_min_age = forms.IntegerField(
+        label=_('Delete Software Snapshots older than (days)'), min_value=0,
+        help_text=_(
+            'Software snapshots older than this will be deleted. '
+            'This does not limit the number of software snapshots created.'))

plinth/modules/snapshot/views.py

@@ -53,10 +53,10 @@ def index(request):
     output = actions.superuser_run('snapshot', ['list'])
     snapshots = json.loads(output)
     has_deletable_snapshots = any(
-        [snapshot for snapshot in snapshots[1:]
+        [snapshot for snapshot in snapshots[1:] if not snapshot['is_default']])
-         if not snapshot['is_default']])
 
-    return TemplateResponse(request, 'snapshot.html', {
+    return TemplateResponse(
+        request, 'snapshot.html', {
             'title': snapshot_module.name,
             'description': snapshot_module.description,
             'snapshots': snapshots,
@@ -65,32 +65,35 @@ def index(request):
         })
 
 
-def update_key_configuration(key, old_status, new_status, stamp,threshold):
-    if old_status[key] != new_status[key]:
-        actions.superuser_run(
-            'snapshot',
-            ['configure', stamp.format(threshold)])
-
-
 def update_configuration(request, old_status, new_status):
     """Update configuration of snapshots."""
-    try:
-        update_key_configuration('enable_timeline_snapshots', old_status, new_status, 'TIMELINE_CREATE={}', yes_or_no(new_status['enable_timeline_snapshots']))
-        update_key_configuration('hourly_limit',old_status,new_status,'TIMELINE_LIMIT_HOURLY={}', new_status['hourly_limit'])
-        update_key_configuration('daily_limit', old_status, new_status, 'TIMELINE_LIMIT_DAILY={}', new_status['daily_limit'])
-        update_key_configuration('weekly_limit', old_status, new_status, 'TIMELINE_LIMIT_WEEKLY={}', new_status['weekly_limit'])
-        update_key_configuration('monthly_limit', old_status, new_status, 'TIMELINE_LIMIT_MONTHLY={}', new_status['monthly_limit'])
-        update_key_configuration('yearly_limit', old_status, new_status, 'TIMELINE_LIMIT_YEARLY={}', new_status['yearly_limit'])
-        update_key_configuration('number_min_age', old_status, new_status, 'NUMBER_MIN_AGE={}', int(new_status['number_min_age'])*86400)
 
+    def update_key_configuration(key, stamp, threshold):
+        if old_status[key] != new_status[key]:
             actions.superuser_run(
-            'snapshot',
+                'snapshot', ['configure', stamp.format(threshold)])
-            ['configure', 'NUMBER_LIMIT=0-0'])
+
-        actions.superuser_run(
+    try:
-            'snapshot',
+        update_key_configuration(
+            'enable_timeline_snapshots', 'TIMELINE_CREATE={}',
+            yes_or_no(new_status['enable_timeline_snapshots']))
+        update_key_configuration('hourly_limit', 'TIMELINE_LIMIT_HOURLY={}',
+                                 new_status['hourly_limit'])
+        update_key_configuration('daily_limit', 'TIMELINE_LIMIT_DAILY={}',
+                                 new_status['daily_limit'])
+        update_key_configuration('weekly_limit', 'TIMELINE_LIMIT_WEEKLY={}',
+                                 new_status['weekly_limit'])
+        update_key_configuration('monthly_limit', 'TIMELINE_LIMIT_MONTHLY={}',
+                                 new_status['monthly_limit'])
+        update_key_configuration('yearly_limit', 'TIMELINE_LIMIT_YEARLY={}',
+                                 new_status['yearly_limit'])
+        update_key_configuration('number_min_age', 'NUMBER_MIN_AGE={}',
+                                 int(new_status['number_min_age']) * 86400)
+
+        actions.superuser_run('snapshot', ['configure', 'NUMBER_LIMIT=0'])
+        actions.superuser_run('snapshot',
                               ['configure', 'NUMBER_LIMIT_IMPORTANT=4-10'])
-        messages.success(request,
+        messages.success(request, _('Storage snapshots configuration updated'))
-                             _('Timeline Snapshots configuration updated'))
     except ActionError as exception:
         messages.error(request,
                        _('Action error: {0} [{1}] [{2}]').format(
@@ -103,7 +106,8 @@ def delete(request, number):
     if request.method == 'POST':
         actions.superuser_run('snapshot', ['delete', number])
         messages.success(
-            request, _('Deleted snapshot #{number}.').format(number=number))
+            request,
+            _('Deleted snapshot #{number}.').format(number=number))
         return redirect(reverse('snapshot:index'))
 
     output = actions.superuser_run('snapshot', ['list'])