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

auto_backup: allow to change the format of backup #1333

Merged
merged 8 commits into from
Aug 10, 2018
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
41 changes: 30 additions & 11 deletions auto_backup/models/db_backup.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,15 @@ class DbBackup(models.Model):
"read permissions for that file.",
)

backup_format = fields.Selection(
[
("zip", "zip (includes filestore)"),
("dump", "pg_dump custom format (without filestore)")
],
default='zip',
help="Choose the format for this backup."
)

@api.model
def _default_folder(self):
"""Default to ``backups`` folder inside current server datadir."""
Expand Down Expand Up @@ -131,11 +140,11 @@ def action_sftp_test_connection(self):
def action_backup(self):
"""Run selected backups."""
backup = None
filename = self.filename(datetime.now())
successful = self.browse()

# Start with local storage
for rec in self.filtered(lambda r: r.method == "local"):
filename = self.filename(datetime.now(), ext=rec.backup_format)
with rec.backup_log():
# Directory must exist
try:
Expand All @@ -151,21 +160,28 @@ def action_backup(self):
shutil.copyfileobj(cached, destiny)
# Generate new backup
else:
db.dump_db(self.env.cr.dbname, destiny)
db.dump_db(
self.env.cr.dbname,
destiny,
backup_format=rec.backup_format
)
backup = backup or destiny.name
successful |= rec

# Ensure a local backup exists if we are going to write it remotely
sftp = self.filtered(lambda r: r.method == "sftp")
if sftp:
if backup:
cached = open(backup)
else:
cached = db.dump_db(self.env.cr.dbname, None)
for rec in sftp:

Choose a reason for hiding this comment

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

@foutoucour the if statement can be removed since you loop on the for.

filename = self.filename(datetime.now(), ext=rec.backup_format)
with rec.backup_log():

with cached:
for rec in sftp:
with rec.backup_log():
cached = db.dump_db(
self.env.cr.dbname,
None,
backup_format=rec.backup_format
)

with cached:
with rec.sftp_connection() as remote:
# Directory must exist
try:
Expand Down Expand Up @@ -255,13 +271,16 @@ def cleanup_log(self):
self.name)

@staticmethod
def filename(when):
def filename(when, ext='zip'):
"""Generate a file name for a backup.

:param datetime.datetime when:
Use this datetime instead of :meth:`datetime.datetime.now`.
:param str ext: Extension of the file. Default: dump.zip
"""
return "{:%Y_%m_%d_%H_%M_%S}.dump.zip".format(when)
return "{:%Y_%m_%d_%H_%M_%S}.{ext}".format(
when, ext='dump.zip' if ext == 'zip' else ext
)

@api.multi
def sftp_connection(self):
Expand Down
26 changes: 13 additions & 13 deletions auto_backup/tests/test_db_backup.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,18 +175,6 @@ def test_action_backup_sftp_remote_open(self):
'wb'
)

def test_action_backup_sftp_remote_open(self):
Copy link
Member

Choose a reason for hiding this comment

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

Why this test was removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello,
I found that it is the same test than the previous test

Copy link
Member

Choose a reason for hiding this comment

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

Your're right

""" It should open remote file w/ proper args """
rec_id = self.new_record()
with self.mock_assets() as assets:
with self.patch_filtered_sftp(rec_id):
conn = rec_id.sftp_connection().__enter__()
rec_id.action_backup()
conn.open.assert_called_once_with(
assets['os'].path.join(),
'wb'
)

def test_action_backup_all_search(self):
""" It should search all records """
rec_id = self.new_record()
Expand Down Expand Up @@ -241,8 +229,20 @@ def test_sftp_connection_return(self, pysftp):
pysftp.Connection(), res,
)

def test_filename(self):
def test_filename_default(self):
""" It should not error and should return a .dump.zip file str """
now = datetime.now()
res = self.Model.filename(now)
self.assertTrue(res.endswith(".dump.zip"))

def test_filename_zip(self):
""" It should return a dump.zip filename"""
now = datetime.now()
res = self.Model.filename(now, ext='zip')
self.assertTrue(res.endswith(".dump.zip"))

def test_filename_dump(self):
""" It should return a dump filename"""
now = datetime.now()
res = self.Model.filename(now, ext='dump')
self.assertTrue(res.endswith(".dump"))
1 change: 1 addition & 0 deletions auto_backup/view/db_backup_view.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
<field name="folder"/>
<field name="days_to_keep"/>
<field name="method"/>
<field name="backup_format"/>
</group>
<div attrs="{'invisible': [('method', '!=', 'sftp')]}">
<div class="bg-warning text-warning">
Expand Down