Python cleanup (#308)

* Clean up all Python imports

* Conform to PEP8: stdlib -> 3rd party -> local ordering
* Alphabetise imports
* Remove some unused imports
* Don't use * imports so that linters can check for missing imports

* Clean up requirements and add separate requirements-dev.txt

* Sort requirements alphabetically
* Add nose as it seems to be what is used to run tests

* Add small note to README about how to run tests

* Rather use pytest for running tests as nose is deprecated

* Add pytest-cov setup for coverage reports

* Use pytest-pep8 to check compliance rather than custom test

* Fix duplicated marathon_lb test names

* Try fix README code block

* Remove unused exception variable

.coveragerc

@@ -0,0 +1,3 @@
+[run]
+source = .
+omit = tests/*

.gitignore

@@ -3,3 +3,10 @@ templates/
 __pycache__
 *.pyc
 .env
+.cache
+
+.coverage
+.coverage.*
+*,cover
+coverage.xml
+htmlcov/

README.md

@@ -342,8 +342,14 @@ PRs are welcome, but here are a few general guidelines:
 
  - Avoid making changes which may break existing behaviour
  - Document new features
- - Update/include tests for new functionality
+ - Update/include tests for new functionality. To install dependencies and run tests:
+
+   ```
+   pip install -r requirements.txt -r requirements-dev.txt
+   pytest --pep8 --cov
+   ```
  - Use the pre-commit hook to automatically generate docs:
+
    ```
    bash /path/to/marathon-lb/scripts/install-git-hooks.sh
-   ```
+   ```

common.py

@@ -1,15 +1,14 @@
 #!/usr/bin/env python3
 
 import json
-import jwt
 import logging
 import os
 import sys
 import time
+from logging.handlers import SysLogHandler
 
-
+import jwt
 import requests
-from logging.handlers import SysLogHandler
 from requests.auth import AuthBase
 
 

config.py

@@ -1,7 +1,7 @@
 #!/usr/bin/env python3
 
-import os
 import logging
+import os
 
 logger = logging.getLogger('marathon_lb')
 

marathon_lb.py

@@ -19,35 +19,37 @@
 
 ### Command Line Usage
 """
-
-from operator import attrgetter
-from shutil import move
-from tempfile import mkstemp
-from wsgiref.simple_server import make_server
-from six.moves.urllib import parse
-from itertools import cycle
-from common import *
-from config import *
-from lrucache import *
-from utils import *
-
 import argparse
+import hashlib
 import json
 import logging
 import os
 import os.path
-import stat
+import random
 import re
-import requests
 import shlex
+import stat
 import subprocess
 import sys
-import time
-import dateutil.parser
 import threading
+import time
 import traceback
-import random
-import hashlib
+from itertools import cycle
+from operator import attrgetter
+from shutil import move
+from tempfile import mkstemp
+from wsgiref.simple_server import make_server
+
+import dateutil.parser
+import requests
+from six.moves.urllib import parse
+
+from common import (get_marathon_auth_params, set_logging_args,
+                    set_marathon_auth_args, setup_logging)
+from config import ConfigTemplater, label_keys
+from lrucache import LRUCache
+from utils import get_task_ip_and_ports, ServicePortAssigner, set_ip_cache
+
 
 logger = logging.getLogger('marathon_lb')
 SERVICE_PORT_ASSIGNER = ServicePortAssigner()
@@ -623,7 +625,7 @@ def get_haproxy_pids():
             "pidof haproxy",
             stderr=subprocess.STDOUT,
             shell=True).split()))
-    except subprocess.CalledProcessError as ex:
+    except subprocess.CalledProcessError:
         return set()
 
 

requirements-dev.txt

@@ -0,0 +1,4 @@
+mock
+pytest
+pytest-cov
+pytest-pep8

requirements.txt

@@ -1,7 +1,5 @@
+cryptography==1.3.2
+PyJWT==1.4.0
+python-dateutil
 requests
 six
-python-dateutil
-pep8
-mock
-PyJWT==1.4.0
-cryptography==1.3.2

tests/test_common.py

@@ -1,8 +1,9 @@
 import unittest
-import common
-import mock
+
 from mock import Mock
 
+import common
+
 
 class TestCommon(unittest.TestCase):
 

tests/test_marathon_lb.py

@@ -1,6 +1,7 @@
-import unittest
 import copy
 import json
+import unittest
+
 import marathon_lb
 
 
@@ -1841,7 +1842,7 @@ def test_config_simple_app_revproxy(self):
 '''
         self.assertMultiLineEqual(config, expected)
 
-    def test_config_simple_app_revproxy(self):
+    def test_config_simple_app_redirect(self):
         apps = dict()
         groups = ['external']
         bind_http_https = True
@@ -2529,7 +2530,7 @@ def test_config_haproxy_map_hybrid_httptohttps_redirect(self):
         expected_app_map["/nginx"] = "nginx_10000"
         self.assertEqual(app_config_map, expected_app_map)
 
-    def test_config_simple_app_proxypass(self):
+    def test_config_simple_app_proxypass_health_check(self):
         apps = dict()
         groups = ['external']
         bind_http_https = True
@@ -2588,7 +2589,7 @@ def test_config_simple_app_proxypass(self):
 '''
         self.assertMultiLineEqual(config, expected)
 
-    def test_config_simple_app_revproxy(self):
+    def test_config_simple_app_revproxy_health_check(self):
         apps = dict()
         groups = ['external']
         bind_http_https = True

tests/test_utils.py

@@ -1,6 +1,8 @@
 import unittest
-import utils
+
 from mock import Mock, patch
+
+import utils
 from utils import ServicePortAssigner
 
 

tests/test_zdd.py

@@ -1,8 +1,10 @@
 import unittest
-import zdd
-import mock
 import json
-from zdd_exceptions import *
+
+import mock
+
+import zdd
+from zdd_exceptions import InvalidArgException
 
 
 class Arguments:

utils.py

@@ -1,11 +1,10 @@
 #!/usr/bin/env python3
 
 import hashlib
-import struct
 import logging
 import socket
 
-from lrucache import *
+from lrucache import LRUCache
 
 logger = logging.getLogger('utils')
 

zdd.py

@@ -1,22 +1,28 @@
 #!/usr/bin/env python3
 
-from common import *
-from datetime import datetime
-from collections import namedtuple
-
 import argparse
-import json
-import requests
 import csv
-import time
+import json
+import logging
 import math
-import six.moves.urllib as urllib
 import socket
-import sys
 import subprocess
-from utils import *
-from zdd_exceptions import *
+import sys
+import time
 import traceback
+from datetime import datetime
+from collections import namedtuple
+
+import requests
+import six.moves.urllib as urllib
+
+from common import (get_marathon_auth_params, set_logging_args,
+                    set_marathon_auth_args, setup_logging)
+from utils import get_task_ip_and_ports
+from zdd_exceptions import (
+    AppCreateException, AppDeleteException, AppScaleException,
+    InvalidArgException, MarathonEndpointException,
+    MarathonLbEndpointException, MissingFieldException)
 
 
 logger = logging.getLogger('zdd')