Use pylint (#30)

* Use pylint and fix some lint failures

* Run linter in travis

* Install dependencies before running linter

* Fix some lint errors; and btw add Gzip() modifier

* refactor all but mysql_source

* fix all lint errors but docstrings

* fix docstrings lint errors

* and I hope a final bit

* python-pip on jessie is broken. let's try easy_install

* add types in some docsrtings

* rename _parent_exists to _full_copy_exists

* retry if boto3 raises NoSuchBucket

* fix linter errors

* fix typo

* Re-try logic in s3.find_files()

* more debug output on s3 re-tries

* Increase retry timeout and raise exeption if it expired

Makefile

@@ -88,16 +88,16 @@ clean-test: ## remove test and coverage artifacts
 clean-docker:
 	@sudo docker rm twindb-backup-build-${PLATFORM}
 
-lint: ## check style with flake8
-	flake8 twindb_backup tests
+lint: test-deps ## check style with flake8
+	pylint twindb_backup
 
 test-deps:
 	pip install --upgrade -r requirements.txt
 	pip install --upgrade -r requirements_dev.txt
 	pip install -U setuptools
 
 test: clean bootstrap ## run tests quickly with the default Python
-	pytest --cov=./twindb_backup tests/unit
+	pytest -xv --cov=./twindb_backup tests/unit
 	codecov
 
 test-integration: test-deps ## run integration tests
@@ -108,7 +108,7 @@ test-all: ## run tests on every Python version with tox
 	tox
 
 coverage: ## check code coverage quickly with the default Python
-	py.test --cov=twindb_backup tests/unit
+	py.test --cov-report term-missing  --cov=twindb_backup tests/unit
 
 docs: ## generate Sphinx HTML documentation, including API docs
 	rm -f docs/twindb_backup.rst

requirements.txt

@@ -5,15 +5,15 @@
 #    pip-compile --no-index --output-file requirements.txt requirements.in
 #
 boto3==1.4.4
-botocore==1.5.5           # via boto3, s3transfer
-click==6.7
+botocore==1.5.10          # via boto3, s3transfer
+Click==6.7
 contextlib2==0.5.4
 docutils==0.13.1          # via botocore
 futures==3.0.5            # via s3transfer
-jmespath==0.9.0           # via boto3, botocore
-psutil==5.0.1
-pyasn1==0.1.9
-pymysql==0.7.9
+jmespath==0.9.1           # via boto3, botocore
+psutil==5.1.3
+pyasn1==0.2.2
+PyMySQL==0.7.9
 python-dateutil==2.6.0    # via botocore
 s3transfer==0.1.10        # via boto3
 six==1.10.0               # via python-dateutil

requirements_dev.in

@@ -9,3 +9,4 @@ pytest-cov
 Sphinx
 unittest2
 moto
+pylint

requirements_dev.txt

@@ -7,45 +7,51 @@
 alabaster==0.7.9          # via sphinx
 appdirs==1.4.0            # via setuptools
 argparse==1.4.0           # via codecov, unittest2
+astroid==1.4.9            # via pylint
 babel==2.3.4              # via sphinx
+backports.functools-lru-cache==1.3  # via pylint
 boto==2.45.0              # via moto
 bumpversion==0.5.3
 codecov==2.0.5
-configparser==3.5.0       # via flake8
+configparser==3.5.0       # via flake8, pylint
 coverage==4.3.4
 docutils==0.13.1          # via sphinx
 enum34==1.1.6             # via flake8
-flake8==3.2.1
+flake8==3.3.0
 funcsigs==1.0.2           # via mock
 httpretty==0.8.10         # via moto
 imagesize==0.7.1          # via sphinx
-jinja2==2.9.4             # via moto, sphinx
+isort==4.2.5              # via pylint
+Jinja2==2.9.5             # via moto, sphinx
+lazy-object-proxy==1.2.2  # via astroid
 linecache2==1.0.0         # via traceback2
-markupsafe==0.23          # via jinja2
-mccabe==0.5.3             # via flake8
+MarkupSafe==0.23          # via jinja2
+mccabe==0.6.1             # via flake8, pylint
 mock==2.0.0
 moto==0.4.31
 packaging==16.8           # via setuptools
 pbr==1.10.0               # via mock
 pluggy==0.4.0             # via tox
 py==1.4.32                # via pytest, tox
-pycodestyle==2.2.0        # via flake8
-pyflakes==1.3.0           # via flake8
-pygments==2.2.0           # via sphinx
+pycodestyle==2.3.1        # via flake8
+pyflakes==1.5.0           # via flake8
+Pygments==2.2.0           # via sphinx
+pylint==1.6.5
 pyparsing==2.1.10         # via packaging
 pytest-cov==2.4.0
 pytest==3.0.6
 python-dateutil==2.6.0    # via moto
 pytz==2016.10             # via babel, moto
 requests==2.13.0          # via codecov, moto, sphinx
-six==1.10.0               # via mock, moto, packaging, python-dateutil, setuptools, sphinx, unittest2
+six==1.10.0               # via astroid, mock, moto, packaging, pylint, python-dateutil, setuptools, sphinx, unittest2
 snowballstemmer==1.2.1    # via sphinx
-sphinx==1.5.2
-tox==2.5.0
+Sphinx==1.5.2
+tox==2.6.0
 traceback2==1.4.0         # via unittest2
 unittest2==1.1.0
 virtualenv==15.1.0        # via tox
 werkzeug==0.11.15         # via moto
+wrapt==1.10.8             # via astroid
 xmltodict==0.10.2         # via moto
 
 # The following packages are considered to be unsafe in a requirements file:

support/docker-test-centos.sh

@@ -27,6 +27,5 @@ timeout=300
 while [ ${timeout} -gt 0 ] ; do mysqladmin ping && break; sleep 1; timeout=$((${timeout} - 1)); done
 
 cd /twindb-backup
-pip install --editable .
 
-make test test-integration
+make lint test test-integration

support/docker-test-debian.sh

@@ -12,8 +12,11 @@ apt-get install -qq \
   make \
   libpython2.7-dev \
   python-setuptools \
-  python-pip \
-  git
+  git \
+  gcc
+
+
+easy_install pip
 
 # install percona repository
 wget https://repo.percona.com/apt/percona-release_0.1-4.$(lsb_release -sc)_all.deb
@@ -33,4 +36,4 @@ while [ ${timeout} -gt 0 ] ; do mysqladmin ping && break; sleep 1; timeout=$((${
 cd /twindb-backup
 pip install --editable .
 
-make test test-integration
+make lint test test-integration

tests/integration/conftest.py

@@ -4,19 +4,19 @@
 
 from subprocess import call
 
-from twindb_backup import log, setup_logging
-from twindb_backup.destination.s3 import S3
+from twindb_backup import LOG, setup_logging
+from twindb_backup.destination.s3 import S3, AWSAuthOptions
 
-setup_logging(log, debug=True)
+setup_logging(LOG, debug=True)
 
 
 @pytest.fixture(scope='session')
 def bucket_name():
     travis_job_number = os.environ.get('TRAVIS_JOB_NUMBER')
-    log.debug('TRAVIS_JOB_NUMBER=%s' % travis_job_number)
+    LOG.debug('TRAVIS_JOB_NUMBER=%s' % travis_job_number)
 
     number = random.randint(0, 1000000)
-    log.debug('Default job number %d' % number)
+    LOG.debug('Default job number %d' % number)
 
     if travis_job_number:
         bucket = 'twindb-backup-test-travis-%s' % travis_job_number
@@ -28,9 +28,11 @@ def bucket_name():
 
 @pytest.fixture(scope='session')
 def s3_client(bucket_name):
-    log.debug('Bucket: %s' % bucket_name)
-    client = S3(bucket_name, os.environ['AWS_ACCESS_KEY_ID'],
-                os.environ['AWS_SECRET_ACCESS_KEY'])
+    LOG.debug('Bucket: %s' % bucket_name)
+    client = S3(bucket_name,
+                AWSAuthOptions(os.environ['AWS_ACCESS_KEY_ID'],
+                               os.environ['AWS_SECRET_ACCESS_KEY'])
+                )
     assert client.create_bucket()
 
     yield client

tests/integration/test_backup.py

@@ -5,8 +5,8 @@
 
 from subprocess import call, Popen, PIPE
 
-from twindb_backup import log
-from twindb_backup.destination.s3 import S3
+from twindb_backup import LOG
+from twindb_backup.destination.s3 import S3, AWSAuthOptions
 
 
 def test__take_file_backup(s3_client, config_content_files_only, tmpdir,
@@ -41,8 +41,8 @@ def test__take_file_backup(s3_client, config_content_files_only, tmpdir,
     proc = Popen(cmd, stdout=PIPE, stderr=PIPE)
     out, err = proc.communicate()
 
-    log.debug('STDOUT: %s' % out)
-    log.debug('STDERR: %s' % err)
+    LOG.debug('STDOUT: %s' % out)
+    LOG.debug('STDERR: %s' % err)
 
     assert proc.returncode == 0
 
@@ -95,8 +95,8 @@ def test__take_mysql_backup(s3_client, config_content_mysql_only, tmpdir):
     proc = Popen(cmd, stdout=PIPE, stderr=PIPE)
     cout, cerr = proc.communicate()
 
-    log.debug('STDOUT: %s', cout)
-    log.debug('STDERR: %s', cerr)
+    LOG.debug('STDOUT: %s', cout)
+    LOG.debug('STDERR: %s', cerr)
 
     key = json.loads(cout)['hourly'].keys()[0]
 
@@ -164,8 +164,10 @@ def test__s3_find_files_returns_sorted(s3_client, config_content_mysql_only,
     for x in xrange(n_runs):
         assert call(cmd) == 0
 
-    dst = S3(s3_client.bucket, os.environ['AWS_ACCESS_KEY_ID'],
-             os.environ['AWS_SECRET_ACCESS_KEY'])
+    dst = S3(s3_client.bucket,
+             AWSAuthOptions(os.environ['AWS_ACCESS_KEY_ID'],
+                            os.environ['AWS_SECRET_ACCESS_KEY'])
+             )
     for x in xrange(10):
         result = dst.find_files(dst.remote_path, 'daily')
         assert len(result) == n_runs

tests/unit/destination/test_s3.py

@@ -4,20 +4,26 @@
 
 from botocore.exceptions import ClientError
 from moto import mock_s3
-from twindb_backup.destination.s3 import S3
+from twindb_backup.destination.s3 import S3, AWSAuthOptions
 
 
 @mock_s3
 def test__create_bucket_creates_the_bucket():
-    s3 = S3('test-bucket', 'access_key', 'secret_key')
+    s3 = S3('test-bucket',
+            AWSAuthOptions('access_key',
+                           'secret_key')
+            )
     s3.create_bucket()
 
     assert s3.s3_client.head_bucket(Bucket='test-bucket')
 
 
 @mock_s3
 def test__delete_bucket_deletes_the_bucket():
-    s3 = S3('test-bucket', 'access_key', 'secret_key')
+    s3 = S3('test-bucket',
+            AWSAuthOptions('access_key',
+                           'secret_key')
+            )
     s3.create_bucket()
 
     s3.delete_bucket()
@@ -28,7 +34,10 @@ def test__delete_bucket_deletes_the_bucket():
 
 @mock_s3
 def test__list_files_returns_sorted_list():
-    s3 = S3('test-bucket', 'access_key', 'secret_key')
+    s3 = S3('test-bucket',
+            AWSAuthOptions('access_key',
+                           'secret_key')
+            )
     s3.create_bucket()
 
     s3.s3_client.put_object(Body='hello world', Bucket='test-bucket',
@@ -40,7 +49,10 @@ def test__list_files_returns_sorted_list():
 
 @mock_s3
 def test__find_files_returns_sorted_list_of_files():
-    s3 = S3('test-bucket', 'access_key', 'secret_key')
+    s3 = S3('test-bucket',
+            AWSAuthOptions('access_key',
+                           'secret_key')
+            )
     s3.create_bucket()
 
     s3.s3_client.put_object(Body='hello world', Bucket='test-bucket',
@@ -57,7 +69,10 @@ def test__find_files_returns_sorted_list_of_files():
 
 @mock_s3
 def test__delete_can_delete_an_object():
-    twindb_s3 = S3('test-bucket', 'access_key', 'secret_key')
+    twindb_s3 = S3('test-bucket',
+                   AWSAuthOptions('access_key',
+                                  'secret_key')
+                   )
     twindb_s3.create_bucket()
 
     twindb_s3.s3_client.put_object(Body='hello world', Bucket='test-bucket',
@@ -76,7 +91,9 @@ def test__delete_can_delete_an_object():
 def test_get_status_empty(mock_status_exists):
     mock_status_exists.return_value = False
 
-    dst = S3('a', 'b', 'c')
+    dst = S3('a',
+             AWSAuthOptions('b',
+                            'c'))
     assert dst.status() == {
         'hourly': {},
         'daily': {},
@@ -87,6 +104,8 @@ def test_get_status_empty(mock_status_exists):
 
 
 def test_basename():
-    dst = S3('bucket', 'b', 'c')
+    dst = S3('bucket',
+             AWSAuthOptions('b',
+                            'c'))
     assert dst.basename('s3://bucket/some_dir/some_file.txt') == \
         'some_dir/some_file.txt'

tests/unit/destination/test_ssh.py

@@ -71,22 +71,19 @@ def test_basename():
         == 'some_dir/some_file.txt'
 
 
-@mock.patch('twindb_backup.destination.ssh.Popen')
-def test_find_files(mock_popen):
-    mock_proc = mock.Mock()
-    mock_proc.communicate.return_value = ('foo', '')
-    mock_proc.returncode = 0
-
-    mock_popen.return_value = mock_proc
+@mock.patch('twindb_backup.destination.ssh.run_command')
+def test_find_files(mock_run_command):
 
     dst = Ssh(remote_path='/foo/bar')
     dst.find_files('/foo/bar', 'abc')
-    mock_popen.assert_called_once_with([
+    mock_run_command.assert_called_once_with([
         'ssh',
         '-l', 'root',
         '-o', 'StrictHostKeyChecking=no',
         '-o', 'PasswordAuthentication=no',
         '-p', '22',
-        '-i', '/root/.id_rsa', '127.0.0.1',
+        '-i', '/root/.id_rsa',
+        '127.0.0.1',
         'find /foo/bar/*/abc -type f'],
-        stderr=PIPE, stdout=PIPE)
+        ok_non_zero=True
+    )

tests/unit/modifiers/test_keeplocal.py

@@ -1,10 +1,11 @@
 from twindb_backup.modifiers.keeplocal import KeepLocal
 
 
-def test_keeplocal(input_file):
+def test_keeplocal(input_file, tmpdir):
+    local_dir = str(tmpdir.join('/foo/bar'))
     with open(str(input_file), 'r') as f:
-        m = KeepLocal(f, '/foo/bar')
-        assert m.local_path == '/foo/bar'
+        m = KeepLocal(f, str(local_dir))
+        assert m.local_path == local_dir
 
 
 def test_keeplocal_saves_file(input_file, tmpdir):