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

Drop support of creating zip backup. #387

Merged
merged 1 commit into from
Jul 13, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion ideascube/conf/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,6 @@


SESSION_COOKIE_AGE = 60 * 60 # Members must be logged out after one hour
BACKUP_FORMAT = 'zip' # One of 'zip', 'tar', 'bztar', 'gztar'
BACKUP_FORMAT = 'gztar' # One of 'tar', 'bztar', 'gztar'
CATALOG_CACHE_BASE_DIR = '/var/cache/ideascube/catalog'
TAGGIT_CASE_INSENSITIVE = True
36 changes: 27 additions & 9 deletions ideascube/serveradmin/backup.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@
from ideascube import __version__


def make_name(format=None):
def make_name(format):
"""Return backup formatted file name."""
format = format or Backup.FORMAT
basename = '-'.join([
settings.IDEASCUBE_ID,
__version__,
Expand All @@ -25,6 +24,7 @@ def make_name(format=None):
class Backup(object):

FORMAT = settings.BACKUP_FORMAT
SUPPORTED_FORMATS_AT_CREATION = ('bztar', 'gztar', 'tar')
Copy link
Member

Choose a reason for hiding this comment

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

Rereviewing this, I'm thinking we could also drop zip restore support: very very fiew backups have been made (I think only by my or so), I think no body is going to try to restore them (it does a backup before restoring, which backup takes ages); and if we really need to restore a zip file, it's at the end only an unzip and put the directory in the right place, which we can do by hand for once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the code is there, it causes no difficulty to keep it, and there might be existing backups we don't know about, I'd say no, let's keep it, at least for some time, to provide a safer transition path.

How about we remove it in a month or two, when we can feel confident that people either have made new tar backups, or they just don't do backups at all?

Copy link
Member

Choose a reason for hiding this comment

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

How about we remove it in a month or two, when we can feel confident that people either have made new tar backups, or they just don't do backups at all?

In a month, nobody but the new deployed boxes will have the new code, but OK ;)

SUPPORTED_FORMATS = ('zip', 'bztar', 'gztar', 'tar')
SUPPORTED_EXTENSIONS = ('.zip', '.tar.bz2', '.tar.gz', '.tar')
FORMAT_TO_EXTENSION = dict(zip(SUPPORTED_FORMATS, SUPPORTED_EXTENSIONS))
Expand Down Expand Up @@ -75,6 +75,9 @@ def format(self):

def save(self):
"""Make a backup of the server data."""
if self.format == 'zip':
raise ValueError(_("Zip is no more supported to create backup"))

return shutil.make_archive(
base_name=os.path.join(Backup.ROOT, self.basename), # W/o ext.
format=self.format,
Expand Down Expand Up @@ -113,6 +116,11 @@ def delete(self):

@classmethod
def create(cls, format=None):
format = format or Backup.FORMAT
if format not in Backup.SUPPORTED_FORMATS_AT_CREATION:
raise ValueError(
_("Format {} is not supported at creation").format(format)
)
name = make_name(format)
backup = Backup(name)
backup.save()
Expand All @@ -136,15 +144,25 @@ def list(cls):
@classmethod
def load(cls, file_):
name = os.path.basename(file_.name)
if name.endswith('.zip'):
if not zipfile.is_zipfile(file_):
raise ValueError(_("Not a zip file"))
elif not tarfile.is_tarfile(file_.name):
raise ValueError(_("Not a tar file"))
file_.seek(0) # Reset cursor.
backup = Backup(name)
with open(backup.path, mode='wb') as f:
f.write(file_.read())
try:
for chunk in file_.chunks():
f.write(chunk)
except AttributeError:
# file_ as no chunks,
# read without them.
while True:
content = file_.read(4096)
if not content:
break
f.write(content)
if not ((name.endswith('.zip') and zipfile.is_zipfile(backup.path))
or tarfile.is_tarfile(backup.path)
):
os.unlink(backup.path)
raise ValueError(_("Not a {} file").format(
'zip' if name.endswith('.zip') else 'tar'))
return backup

@classmethod
Expand Down
13 changes: 8 additions & 5 deletions ideascube/serveradmin/management/commands/backup.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,27 +21,30 @@ def add_arguments(self, parser):
'"add", "delete" or "restore" action.')
parser.add_argument('--format', default=None,
help='Backup file format. Possible values: {}'
'Default: {}'.format(Backup.SUPPORTED_FORMATS,
'Default: {}'.format(Backup.SUPPORTED_FORMATS_AT_CREATION,
Backup.FORMAT))

def handle(self, *args, **options):
format = options['format']
if format and format not in Backup.SUPPORTED_FORMATS:
action = options['action']
supported_formats = (Backup.SUPPORTED_FORMATS_AT_CREATION
if action == 'create'
else Backup.SUPPORTED_FORMATS )
if format and format not in supported_formats:
self.stderr.write('Unsupported format: {}'.format(format))
sys.exit(1)
action = options['action']
reference = options['reference']
interactive = options.get('interactive')
if action == 'list':
self.stdout.write('* Available backups:')
for backup in Backup.list():
self.stdout.write(str(backup))
elif action == 'create':
backup = Backup.create(format=options['format'])
backup = Backup.create(format=format)
self.stdout.write('Succesfully created backup {}'.format(backup))
elif action == 'add':
if not reference:
self.stderr.write('Missing path to zip backup to add.')
self.stderr.write('Missing path to archive backup to add.')
sys.exit(1)
if not os.path.exists(reference):
self.stderr.write('File not found {}'.format(reference))
Expand Down
2 changes: 1 addition & 1 deletion ideascube/serveradmin/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@
def backup(monkeypatch):
monkeypatch.setattr('ideascube.serveradmin.backup.Backup.ROOT', DATA_ROOT)
# ZIP file should be shipped by git in serveradmin/tests/data
return Backup('musasa-0.1.0-201501241620.zip')
return Backup('musasa-0.1.0-201501241620.tar.gz')
63 changes: 24 additions & 39 deletions ideascube/serveradmin/tests/test_backup.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import os
import tarfile
import zipfile

import pytest

Expand All @@ -19,7 +18,7 @@ def test_backup_init_should_raise_with_malformatted_string():


def test_underscore_as_separator_should_still_be_supported():
Backup('edoardo-0.0.0-201501231405.zip')
Backup('edoardo-0.0.0-201501231405.tar.gz')


@pytest.mark.parametrize('input,expected', [
Expand All @@ -36,9 +35,9 @@ def test_list(monkeypatch, settings):
monkeypatch.setattr('ideascube.serveradmin.backup.Backup.ROOT',
BACKUPS_ROOT)
settings.DATETIME_FORMAT = 'Y'
filename = 'edoardo-0.0.0-201501231405.zip'
filename = 'edoardo-0.0.0-201501231405.tar'
good = os.path.join(BACKUPS_ROOT, filename)
bad = os.path.join(BACKUPS_ROOT, 'donotlistme.zip')
bad = os.path.join(BACKUPS_ROOT, 'donotlistme.tar')
try:
os.makedirs(BACKUPS_ROOT)
except OSError:
Expand All @@ -53,35 +52,6 @@ def test_list(monkeypatch, settings):
os.rmdir(BACKUPS_ROOT)


def test_create_zipfile(monkeypatch, settings):
settings.BACKUPED_ROOT = BACKUPED_ROOT
try:
os.makedirs(BACKUPED_ROOT)
except OSError:
pass
filename = 'edoardo-0.0.0-201501231405.zip'
filepath = os.path.join(BACKUPS_ROOT, filename)
try:
# Make sure it doesn't exist before running backup.
os.remove(filepath)
except OSError:
pass
monkeypatch.setattr('ideascube.serveradmin.backup.Backup.FORMAT', 'zip')
monkeypatch.setattr('ideascube.serveradmin.backup.Backup.ROOT',
BACKUPS_ROOT)
monkeypatch.setattr('ideascube.serveradmin.backup.make_name',
lambda f: filename)
proof_file = os.path.join(settings.BACKUPED_ROOT, 'backup.me')
open(proof_file, mode='w')
Backup.create()
assert os.path.exists(filepath)
assert zipfile.is_zipfile(filepath)
archive = zipfile.ZipFile(filepath)
assert 'backup.me' in archive.namelist()
os.remove(filepath)
os.remove(proof_file)


@pytest.mark.parametrize('format,extension', [
('tar', '.tar'),
('bztar', '.tar.bz2'),
Expand Down Expand Up @@ -117,6 +87,15 @@ def test_create_tarfile(monkeypatch, settings, format, extension):
os.remove(proof_file)


def test_create_zipfile_must_fail(monkeypatch, tmpdir):
monkeypatch.setattr('ideascube.serveradmin.backup.Backup.ROOT', str(tmpdir))
monkeypatch.setattr('ideascube.serveradmin.backup.Backup.FORMAT', 'zip')
assert len(list(Backup.list())) == 0
with pytest.raises(ValueError) as excinfo:
Backup.create()
assert len(list(Backup.list())) == 0


@pytest.mark.parametrize('extension', [
('.zip'),
('.tar'),
Expand Down Expand Up @@ -154,10 +133,16 @@ def test_load(monkeypatch, extension):
os.remove(backup_path)


def test_delete(monkeypatch):
@pytest.mark.parametrize('extension', [
('.zip'),
('.tar'),
('.tar.gz'),
('.tar.bz2'),
])
def test_delete(monkeypatch, extension):
monkeypatch.setattr('ideascube.serveradmin.backup.Backup.ROOT',
BACKUPS_ROOT)
backup_name = 'musasa-0.1.0-201501241620.zip'
backup_name = 'musasa-0.1.0-201501241620' + extension
backup_path = os.path.join(BACKUPS_ROOT, backup_name)
assert not os.path.exists(backup_path)
with open(os.path.join(DATA_ROOT, backup_name), mode='rb') as f:
Expand All @@ -170,13 +155,13 @@ def test_delete(monkeypatch):
def test_delete_should_not_fail_if_file_is_missing(monkeypatch):
monkeypatch.setattr('ideascube.serveradmin.backup.Backup.ROOT',
BACKUPS_ROOT)
backup = Backup('doesnotexist-0.1.0-201501241620.zip')
backup = Backup('doesnotexist-0.1.0-201501241620.tar')
backup.delete()


def test_load_should_raise_if_file_is_not_a_zip():
with pytest.raises(ValueError) as excinfo:
Backup.load(ContentFile('xxx', name='musasa_0.1.0_201501241620.zip'))
Backup.load(ContentFile(b'xxx', name='musasa_0.1.0_201501241620.zip'))
assert 'Not a zip file' in str(excinfo.value)


Expand All @@ -191,5 +176,5 @@ def test_load_should_raise_if_file_is_not_a_tar():

def test_exists(monkeypatch, settings):
monkeypatch.setattr('ideascube.serveradmin.backup.Backup.ROOT', DATA_ROOT)
assert Backup.exists('musasa-0.1.0-201501241620.zip')
assert not Backup.exists('doesnotexist.zip')
assert Backup.exists('musasa-0.1.0-201501241620.tar')
assert not Backup.exists('doesnotexist.tar')
33 changes: 18 additions & 15 deletions ideascube/serveradmin/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from io import BytesIO
import os
import shutil
import zipfile
import tarfile

import pytest
from webtest.forms import Checkbox, Upload
Expand Down Expand Up @@ -231,20 +231,20 @@ def test_backup_should_list_available_backups(staffapp, backup):
form = staffapp.get(reverse('server:backup')).forms['backup']
radio_options = form.get('backup').options
assert len(radio_options) == 4
assert radio_options[3][0] == backup.name
assert radio_options[2][0] == backup.name


def test_backup_button_should_save_a_new_backup(staffapp, monkeypatch,
settings):
monkeypatch.setattr('ideascube.serveradmin.backup.Backup.FORMAT', 'zip')
monkeypatch.setattr('ideascube.serveradmin.backup.Backup.FORMAT', 'gztar')
settings.BACKUPED_ROOT = BACKUPED_ROOT
try:
os.makedirs(BACKUPED_ROOT)
except OSError:
pass
if os.path.exists(BACKUPS_ROOT):
shutil.rmtree(BACKUPS_ROOT)
filename = 'edoardo-0.0.0-201501231405.zip'
filename = 'edoardo-0.0.0-201501231405.tar.gz'
filepath = os.path.join(BACKUPS_ROOT, filename)
try:
# Make sure it doesn't exist before running backup.
Expand All @@ -260,9 +260,9 @@ def test_backup_button_should_save_a_new_backup(staffapp, monkeypatch,
form = staffapp.get(reverse('server:backup')).forms['backup']
form.submit('do_create')
assert os.path.exists(filepath)
assert zipfile.is_zipfile(filepath)
archive = zipfile.ZipFile(filepath)
assert 'backup.me' in archive.namelist()
assert tarfile.is_tarfile(filepath)
archive = tarfile.open(filepath)
assert './backup.me' in archive.getnames()
os.remove(filepath)
os.remove(proof_file)

Expand All @@ -282,19 +282,22 @@ def test_restore_button_should_restore(staffapp, monkeypatch, settings,
os.remove(dbpath)


def test_download_button_should_download_zip_file(staffapp, backup):
def test_download_button_should_download_tar_file(staffapp, backup, tmpdir):
form = staffapp.get(reverse('server:backup')).forms['backup']
form['backup'] = backup.name
resp = form.submit('do_download')
assert backup.name in resp['Content-Disposition']
assert zipfile.is_zipfile(ContentFile(resp.content))
archive_name = os.path.join(str(tmpdir), 'backup.tar')
with open(archive_name, mode="wb") as f:
f.write(resp.content)
assert tarfile.is_tarfile(archive_name)


def test_delete_button_should_remove_file(staffapp, backup):
assert len(os.listdir(DATA_ROOT)) == 4
with open(backup.path, mode='rb') as f:
other = Backup.load(ContentFile(f.read(),
name='kavumu-0.1.0-201401241620.zip'))
name='kavumu-0.1.0-201401241620.tar.gz'))
assert len(os.listdir(DATA_ROOT)) == 5
form = staffapp.get(reverse('server:backup')).forms['backup']
form['backup'] = other.name
Expand All @@ -308,7 +311,7 @@ def test_upload_button_create_new_backup_with_uploaded_file(staffapp,
BACKUPS_ROOT)
if os.path.exists(BACKUPS_ROOT):
shutil.rmtree(BACKUPS_ROOT)
backup_name = 'musasa-0.1.0-201501241620.zip'
backup_name = 'musasa-0.1.0-201501241620.tar.gz'
backup_path = os.path.join(BACKUPS_ROOT, backup_name)
assert not os.path.exists(backup_path)
with open(os.path.join(DATA_ROOT, backup_name), mode='rb') as f:
Expand All @@ -319,11 +322,11 @@ def test_upload_button_create_new_backup_with_uploaded_file(staffapp,
os.remove(backup_path)


def test_upload_button_do_not_create_backup_with_non_zip_file(staffapp,
def test_upload_button_do_not_create_backup_with_non_tar_file(staffapp,
monkeypatch):
monkeypatch.setattr('ideascube.serveradmin.backup.Backup.ROOT',
BACKUPS_ROOT)
backup_name = 'musasa-0.1.0-201501241620.zip'
backup_name = 'musasa-0.1.0-201501241620.tar'
backup_path = os.path.join(BACKUPS_ROOT, backup_name)
assert not os.path.exists(backup_path)
form = staffapp.get(reverse('server:backup')).forms['backup']
Expand All @@ -337,12 +340,12 @@ def test_upload_button_do_not_create_backup_with_bad_file_name(staffapp,
monkeypatch):
monkeypatch.setattr('ideascube.serveradmin.backup.Backup.ROOT',
BACKUPS_ROOT)
backup_name = 'musasa-0.1.0-201501241620.zip'
backup_name = 'musasa-0.1.0-201501241620.tar'
backup_path = os.path.join(BACKUPS_ROOT, backup_name)
assert not os.path.exists(backup_path)
with open(os.path.join(DATA_ROOT, backup_name), mode='rb') as f:
form = staffapp.get(reverse('server:backup')).forms['backup']
form['upload'] = Upload('badname.zip', f.read())
form['upload'] = Upload('badname.tar', f.read())
resp = form.submit('do_upload')
assert resp.status_code == 200
assert not os.path.exists(backup_path)
Expand Down