-
Notifications
You must be signed in to change notification settings - Fork 4
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
BackupDB 1.0 #11
Open
acatton
wants to merge
3
commits into
master
Choose a base branch
from
refactoring
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
BackupDB 1.0 #11
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,2 @@ | ||
include README.rst | ||
include LICENSE | ||
include test_settings.py |
File renamed without changes.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
# -*- coding: utf-8 -*- | ||
import os | ||
import abc | ||
|
||
import spm | ||
import six | ||
|
||
|
||
class BaseBackend(six.with_metaclass(abc.ABCMeta, object)): | ||
def __init__(self, **kwargs): | ||
self.db_config = kwargs.pop('db_config') | ||
self.backup_file = kwargs.pop('backup_file') | ||
self.extra_args = kwargs.pop('extra_args', []) | ||
self.show_ouput = kwargs.pop('show_ouput', False) | ||
|
||
@abc.abstractmethod | ||
def get_backup_command(self, db_name): | ||
""" | ||
Returns a spm command which dumps the database on stdout | ||
""" | ||
|
||
@abc.abstractmethod | ||
def get_restore_command(self, db_name, drop_tables): | ||
""" | ||
Returns a spm command which takes a backup file on stdin | ||
""" | ||
|
||
def do_backup(self): | ||
db_name = self.db_config['NAME'] # This has to raise KeyError if it does | ||
# not exist. | ||
|
||
command = self.get_backup_command(db_name) | ||
|
||
proc = command.pipe('gzip') | ||
proc.stdout = open(self.backup_file, 'w') | ||
proc.wait() | ||
|
||
def do_restore(self, drop_tables): | ||
db_name = self.db_config['NAME'] # This has to raise KeyError if it does | ||
# not exist. | ||
|
||
if not os.path.isfile(self.backup_file): | ||
raise RuntimeError("{} does not exist".format(fname)) | ||
|
||
command = self.get_restore_command(db_name, drop_tables=drop_tables) | ||
|
||
proc = spm.run('zcat', self.backup_file).pipe(command) | ||
proc.wait() |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
from __future__ import absolute_import | ||
import os | ||
|
||
import spm | ||
|
||
from backupdb.utils import apply_arg_values | ||
from backupdb.backends.base import BaseBackend | ||
|
||
|
||
class MySQLBackend(BaseBackend): | ||
extension = 'mysql' | ||
|
||
def get_arguments(self): | ||
return apply_arg_values( | ||
('--user={}', self.db_config.get('USER')), | ||
('--password={}', self.db_config.get('PASSWORD')), | ||
('--host={}', self.db_config.get('HOST')), | ||
('--port={}', self.db_config.get('PORT')) | ||
) | ||
|
||
def get_backup_command(self, db_name): | ||
arguments = ['mysqldump'] + self.get_arguments() + self.extra_args + \ | ||
[db_name] | ||
return spm.run(*arguments) | ||
|
||
def get_restore_command(self, db_name, drop_tables): | ||
# TODO: Drop all tables from the database if drop_tables is True | ||
|
||
arguments = ['mysql'] + self.get_arguments() + self.extra_args + \ | ||
[db_name] | ||
return spm.run(*arguments) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
from __future__ import absolute_import | ||
import os | ||
|
||
import spm | ||
|
||
from backupdb.utils import apply_arg_values | ||
from backupdb.backends.base import BaseBackend | ||
|
||
# TODO: Use this command to drop tables from a database | ||
PG_DROP_SQL = """SELECT format('DROP TABLE IF EXISTS %I CASCADE;', tablename) | ||
FROM pg_tables WHERE schemaname = 'public';""" | ||
|
||
|
||
class PostgreSQLBackend(BaseBackend): | ||
extension = 'pgsql' | ||
|
||
def get_env(self): | ||
try: | ||
return {'PGPASSWORD': self.db_config['PASSWORD']} | ||
except KeyError: | ||
return {} | ||
|
||
def get_arguments(self): | ||
return apply_arg_values( | ||
('--username={0}', self.db_config.get('USER')), | ||
('--host={0}', self.db_config.get('HOST')), | ||
('--port={0}', self.db_config.get('PORT')), | ||
) | ||
|
||
def get_backup_command(self): | ||
arguments = ['pg_dump', '--clean'] + self.get_arguments() + \ | ||
self.extra_args + [database] | ||
env = self.get_env() | ||
return spm.run(*arguments, env=env) | ||
|
||
def get_restore_command(self, drop_tables): | ||
# TODO: Drop all tables from the database if drop_tables is True | ||
database = self.db_config['NAME'] | ||
|
||
if not os.path.isfile(self.backup_file): | ||
raise RuntimeError("{} does not exist".format(fname)) | ||
|
||
arguments = ['psql'] + self.get_arguments() + self.extra_args + \ | ||
[database] | ||
env = self.get_env() | ||
|
||
return spm.run(*arguments, env=env) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
from __future__ import absolute_import | ||
|
||
import spm | ||
|
||
from backupdb.backends.base import BaseBackend | ||
|
||
|
||
class SQLite3Backend(BaseBackend): | ||
extension = 'sqlite' | ||
|
||
def get_backup_command(self, fname): | ||
arguments = ['sqlite3'] + self.extra_args + [fname, '.dump'] | ||
command = spm.run(*arguments) | ||
|
||
return command | ||
|
||
def get_restore_command(self, fname, drop_tables): | ||
# TODO: Drop all tables from the database if drop_tables is True | ||
return spm.run('sqlite3', fname) |
File renamed without changes.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be wrapped in an
os.path.abspath
? It doesn't appear thatget_backup_directory
returns an absolute path.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, you're totally right. I'm just not sure where to get the absolute path. Should it be
os.path.join(PROJECT_PATH, BACKUP_DIR)
, becauseBACKUP_DIRECTORY
doesn't have to be an absolute directory.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since
os.path.abspath
is idempotent, why don't you just pass the output ofget_backup_directory
through it to ensure it's absolute.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/django/django/blob/master/django/conf/project_template/project_name/settings.py#L16
PROJECT_PATH
is more of a Fusionbox concept, but most django installs probably haveBASE_DIR
.That being said. I would much rather have the user decide that and ask that they provide absolute URLs instead of us guessing what things they have in their settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You also might consider calling
os.path.expanduser
before callingabspath
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rockymeza I don't think it being relative now will matter. Is there something specific that you're concerned about expanding relative paths to absolute paths?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of the builtin Django settings are absolute, like MEDIA_ROOT, etc.
I'm just opposed to guessing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@acatton When you say guessing, are you referring to using
backups/
as the default?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
abspath
seems like a good idea.But I don't think we should call
os.path.expanduser
. If you want to do it in your settingsBACKUP_DIRECTORY = os.path.expanduser(yourpath)
, that's fine. What if I want my backups to go in'~backups/'
in my project?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See fc7d478