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 4 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
29 changes: 18 additions & 11 deletions auto_backup/models/db_backup.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ class DbBackup(models.Model):
"read permissions for that file.",
)

backup_format = fields.Selection(
[("dump.zip", "zip (includes filestore)"), ("dump", "pg_dump custom format (without filestore)")],
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't be "zip" here? I'm wondering how this functionality could work fine with "dump.zip" passed to dump_db() 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@astirpe fixed!

string="Backup Format",
Copy link
Member

Choose a reason for hiding this comment

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

No need to write string="Backup Format",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Question of style. I prefer explicit declaration that implicit declaration.
Has OCA guide on this that I didn't see?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

default='dump.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 +138,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 +158,20 @@ 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():

cached = db.dump_db(self.env.cr.dbname, None, backup_format=rec.backup_format)

with cached:
for rec in sftp:
with rec.backup_log():
with cached:
with rec.sftp_connection() as remote:
# Directory must exist
try:
Expand Down Expand Up @@ -255,13 +261,14 @@ def cleanup_log(self):
self.name)

@staticmethod
def filename(when):
def filename(when, ext='dump.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=ext)

@api.multi
def sftp_connection(self):
Expand Down
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
29 changes: 29 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
version: '3'
Copy link
Member

Choose a reason for hiding this comment

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

Could you remove file docker-compose.yml from this PR?

services:
odoo:
image: quay.io/numigi/odoo-base:11.12
volumes:
- odoo-web-data:/var/lib/odoo
- ./log:/var/log/odoo
- ./auto_backup:/mnt/extra-addons/auto_backup
ports:
- "127.0.0.1:8069:8069"
- "127.0.0.1:8071:8071"
depends_on:
- db
environment:
- LOG_ODOO=/var/log/odoo
command: odoo
db:
image: quay.io/numigi/postgres-odoo:11.1
environment:
- POSTGRES_PASSWORD=odoo
- POSTGRES_USER=odoo
- PGDATA=/var/lib/postgresql/data/pgdata
volumes:
- odoo-db-data:/var/lib/postgresql/data/pgdata
expose:
- 5432
volumes:
odoo-web-data:
odoo-db-data: