Backups: minor cleanups

- some variable names, indentation and documentation changes
- removed unused backups action
- changed name of upload session variable to 'fbx-backups-upload-path'

Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

actions/backups

@@ -23,7 +23,6 @@ Configuration helper for backups.
 import argparse
 import json
 import os
-import shutil
 import subprocess
 import sys
 import tarfile
@@ -49,20 +48,9 @@ def parse_arguments():
     delete = subparsers.add_parser('delete', help='Delete archive')
     delete.add_argument('--name', help='Archive name', required=True)
 
-    extract = subparsers.add_parser('extract', help='Extract archive contents')
-    extract.add_argument('--name', help='Archive name', required=True)
-    extract.add_argument('--destination', help='Extract destination',
-                         required=True)
-
-    export_tar = subparsers.add_parser('export-tar',
-                                   help='Export archive contents as tarball')
-    export_tar.add_argument('--name', help='Archive name)', required=True)
-    export_tar.add_argument('--filepath', help='Destination tarball file path',
-                            required=True)
-
-    export_stream = subparsers.add_parser('export-stream',
-                               help='Export archive contents as tar stream')
-    export_stream.add_argument('--name', help='Archive name)',
+    export_help='Export archive contents as tar on stdout'
+    export_tar = subparsers.add_parser('export-tar', help=export_help)
+    export_tar.add_argument('--name', help='Archive name)',
                                required=True)
 
     get_exported_archive_apps = subparsers.add_parser(
@@ -132,12 +120,6 @@ def subcommand_delete(arguments):
                    check=True)
 
 
-def subcommand_extract(arguments):
-    """Extract archive contents."""
-    path = REPOSITORY + '::' + arguments.name
-    return _extract(path, arguments.destination)
-
-
 def _extract(archive_path, destination, locations=None):
     """Extract archive contents."""
     prev_dir = os.getcwd()
@@ -164,24 +146,7 @@ def _extract(archive_path, destination, locations=None):
 
 
 def subcommand_export_tar(arguments):
-    """Export archive contents as tarball."""
-    directory, filename = os.path.split(arguments.filepath)
-    if not os.path.exists(directory):
-        os.makedirs(directory)
-
-    subprocess.run([
-        'borg', 'export-tar', REPOSITORY + '::' + arguments.name,
-        arguments.filepath
-    ], check=True)
-
-    try:
-        shutil.chown(arguments.filename, user='plinth', group='plinth')
-    except PermissionError:
-        pass
-
-
-def subcommand_export_stream(arguments):
-    """Export archive contents as tar stream."""
+    """Export archive contents as tar stream on stdout."""
     subprocess.run([
         'borg', 'export-tar', REPOSITORY + '::' + arguments.name, '-'],
         check=True)

plinth/actions.py

@@ -185,10 +185,6 @@ def _run(action, options=None, input=None, run_in_background=False,
 
     LOGGER.info('Executing command - %s', cmd)
 
-    # Use default bufsize if no bufsize is given.
-    if bufsize is None:
-        bufsize = -1
-
     # Contract 3C: don't interpret shell escape sequences.
     # Contract 5 (and 6-ish).
     kwargs = {
@@ -196,11 +192,12 @@ def _run(action, options=None, input=None, run_in_background=False,
         "stdout": subprocess.PIPE,
         "stderr": subprocess.PIPE,
         "shell": False,
-        "bufsize": bufsize,
     }
     if cfg.develop:
         # In development mode pass on local pythonpath to access Plinth
         kwargs['env'] = {'PYTHONPATH': cfg.root}
+    if bufsize is not None:
+        kwargs['bufsize'] = bufsize
     proc = subprocess.Popen(cmd, **kwargs)
 
     if not run_in_background:

plinth/modules/backups/__init__.py

@@ -44,7 +44,7 @@ service = None
 MANIFESTS_FOLDER = '/var/lib/plinth/backups-manifests/'
 REPOSITORY = '/var/lib/freedombox/borgbackup'
 # session variable name that stores when a backup file should be deleted
-SESSION_BACKUP_PATH = 'fbx-backup-path'
+SESSION_PATH_VARIABLE = 'fbx-backups-upload-path'
 
 
 def init():
@@ -106,16 +106,6 @@ def delete_archive(name):
     actions.superuser_run('backups', ['delete', '--name', name])
 
 
-def export_archive(name, filepath):
-    """Export an archive as .tar.gz file
-
-    name: name of the repository (w/o path)
-    filepath: filepath the archive should be exported to
-    """
-    arguments = ['export-tar', '--name', name, '--filepath', filepath]
-    actions.superuser_run('backups', arguments)
-
-
 def get_archive_path(archive_name):
     """Get path of an archive"""
     return "::".join([REPOSITORY, archive_name])
@@ -152,7 +142,7 @@ def _restore_archive_handler(packet):
 
 
 def restore_from_upload(path, apps=None):
-    """Restore files from (uploaded) eported backup file"""
+    """Restore files from an uploaded .tar.gz backup file"""
     api.restore_apps(_restore_exported_archive_handler, app_names=apps,
                      create_subvolume=False, backup_file=path)
 

plinth/modules/backups/decorators.py

@@ -20,18 +20,18 @@ Decorators for the backup views.
 
 import os
 
-from . import SESSION_BACKUP_PATH
+from . import SESSION_PATH_VARIABLE
 
 
 def delete_tmp_backup_file(function):
     """Decorator to delete uploaded backup files"""
 
     def wrap(request, *args, **kwargs):
-        path = request.session.get(SESSION_BACKUP_PATH, None)
+        path = request.session.get(SESSION_PATH_VARIABLE, None)
         if path: 
             if os.path.isfile(path):
                 os.remove(path)
-            del request.session[SESSION_BACKUP_PATH]
+            del request.session[SESSION_PATH_VARIABLE]
         return function(request, *args, **kwargs)
 
     wrap.__doc__ = function.__doc__

plinth/modules/backups/templates/backups_upload.html

@@ -28,19 +28,19 @@
 
   <h3>{{ title }}</h3>
   <p>
-  {% blocktrans %}
-  You can choose the apps you wish to import after uploading a backup file.
-  {% endblocktrans %}
+    {% blocktrans %}
+      You can choose the apps you wish to import after uploading a backup file.
+    {% endblocktrans %}
   </p>
   <br />
 
   <form class="form" enctype="multipart/form-data" method="post">
-      {% csrf_token %}
+    {% csrf_token %}
 
-      {{ form|bootstrap }}
+    {{ form|bootstrap }}
 
-      <input type="submit" class="btn btn-primary"
-             value="{% trans "Upload file" %}"/>
+    <input type="submit" class="btn btn-primary"
+           value="{% trans "Upload file" %}"/>
   </form>
 
 {% endblock %}

plinth/modules/backups/views.py

@@ -39,7 +39,7 @@ from django.views.generic import View, FormView, TemplateView
 from plinth.modules import backups
 from plinth import actions
 
-from . import api, forms, SESSION_BACKUP_PATH
+from . import api, forms, SESSION_PATH_VARIABLE
 from .decorators import delete_tmp_backup_file
 
 subsubmenu = [{
@@ -147,7 +147,7 @@ class UploadArchiveView(SuccessMessageMixin, FormView):
     def form_valid(self, form):
         """store uploaded file."""
         with tempfile.NamedTemporaryFile(delete=False) as tmp_file:
-            self.request.session[SESSION_BACKUP_PATH] = tmp_file.name
+            self.request.session[SESSION_PATH_VARIABLE] = tmp_file.name
             for chunk in self.request.FILES['backups-file'].chunks():
                 tmp_file.write(chunk)
         return super().form_valid(form)
@@ -183,7 +183,7 @@ class RestoreFromUploadView(BaseRestoreView):
     """View to restore files from an (uploaded) exported archive."""
 
     def get(self, *args, **kwargs):
-        path = self.request.session.get(SESSION_BACKUP_PATH)
+        path = self.request.session.get(SESSION_PATH_VARIABLE)
         if not os.path.isfile(path):
             messages.error(self.request, _('No backup file found.'))
             return redirect(reverse_lazy('backups:index'))
@@ -198,12 +198,12 @@ class RestoreFromUploadView(BaseRestoreView):
 
     def _get_included_apps(self):
         """Save some data used to instantiate the form."""
-        path = self.request.session.get(SESSION_BACKUP_PATH)
+        path = self.request.session.get(SESSION_PATH_VARIABLE)
         return backups.get_exported_archive_apps(path)
 
     def form_valid(self, form):
         """Restore files from the archive on valid form submission."""
-        path = self.request.session.get(SESSION_BACKUP_PATH)
+        path = self.request.session.get(SESSION_PATH_VARIABLE)
         backups.restore_from_upload(path, form.cleaned_data['selected_apps'])
         return super().form_valid(form)
 
@@ -230,7 +230,7 @@ class ZipStream(object):
     def __init__(self, stream, get_chunk_method):
         """
         - stream: the input stream
-        - get_chunk_method: name of the method that yields chunks
+        - get_chunk_method: name of the method to get a chunk of the stream
         """
         self.stream = stream
         self.buffer = BytesIO()
@@ -257,11 +257,12 @@ class ExportAndDownloadView(View):
     def get(self, request, name):
         name = unquote(name)
         filename = "%s.tar.gz" % name
-        args = ['export-stream', '--name', name]
+        args = ['export-tar', '--name', name]
         proc = actions.superuser_run('backups', args, run_in_background=True,
                 bufsize=1)
         zipStream = ZipStream(proc.stdout, 'readline')
         response = StreamingHttpResponse(zipStream,
                 content_type="application/x-gzip")
-        response['Content-Disposition'] = 'attachment; filename="%s"' % filename
+        response['Content-Disposition'] = 'attachment; filename="%s"' % \
+                filename
         return response