Merge pull request #43 from d3ming/commandline

Commandline support

.codeclimate.yml

@@ -2,7 +2,7 @@ engines:
   radon:
     enabled: true
     config:
-      threshold: 'A'
+      threshold: 'B'
   fixme:
     enabled: true
   pep8:

Makefile

@@ -1,5 +1,5 @@
 install:
-	pip install -r requirements.txt -q
+	pip install -e .
 
 .PHONY: flake
 flake: install

README.md

@@ -44,8 +44,16 @@ Update the `settings.local.json` file with the desired input to the tool as desi
 `tagcompare` is the main entry point for the tool, it will capture and compare tags 
 with the given input in your `settings.local.json`
 
-All outputs generated by the tool will be stored in `/tmp/tagcompare`
+### Commandline arguments:
+Commandline arguments can optionally be used to configure the tagcompare run.
+`tagcompare --help` will show the current list of supported options.
+
+Note:Options passed through the commandline will override the `settings.local.json`
+
+### Outputs:
+Outputs generated by the tool will be stored in `/~/tagcompare`
 
 ## Running tests
-`make test` or `tox` will run unit tests
-`make test-all` will run all the tests
+`make test` or `tox` runs unit tests
+`make test-all` runs all the tests
+`./test <path_to_test>` runs tests from a specific test file

setup.py

@@ -1,4 +1,5 @@
 import sys
+import subprocess
 
 from setuptools import setup
 from setuptools.command.test import test as TestCommand
@@ -28,9 +29,20 @@ def run_tests(self):
         sys.exit(errno)
 
 
+def git_version():
+    try:
+        version = subprocess.check_output(
+            'git describe --always --dirty'.split()
+        ).strip().decode()
+    except subprocess.CalledProcessError:
+        version = '0.0.0'
+
+    return version
+
+
 setup(
     name='tagcompare',
-    version='0.1.1',
+    version=git_version(),
     description='Capture and compare creative tags!',
     url='https://github.com/paperg/tagcompare',
     packages=['tagcompare', 'tagcompare.test'],
@@ -44,7 +56,8 @@ def run_tests(self):
         'selenium',
         'requests',
         'pillow',
-        'enum34'
+        'enum34',
+        'coveralls'
     ],
     package_data={
         '': ['*.json'],

tagcompare/capture.py

@@ -149,17 +149,21 @@ def __capture_tags(capabilities, tags, pathbuilder,
     return browser_errors
 
 
-def main(cids=settings.DEFAULT.campaigns, pids=settings.DEFAULT.publishers):
+def main():
     """
     Runs capture, returns the job name for the capture job
     :param cids:
     :param pids:
     :return:
     """
+
     original_build = output.generate_build_string()
     build = "capture_" + original_build
     pathbuilder = output.create(build=build)
-    cids = placelocal.get_cids(cids=cids, pids=pids)
+    cids = placelocal.get_cids_from_settings()
+    LOGGER.info("Starting capture against %s for %s campaigns: %s...",
+                settings.DEFAULT.domain,
+                len(cids), cids)
     output.aggregate()
 
     configs = settings.DEFAULT.configs_in_comparisons()
@@ -168,4 +172,4 @@ def main(cids=settings.DEFAULT.campaigns, pids=settings.DEFAULT.publishers):
 
 
 if __name__ == '__main__':
-    main(cids=settings.DEFAULT.campaigns, pids=settings.DEFAULT.publishers)
+    main()

tagcompare/compare.py

@@ -13,7 +13,7 @@
 NUM_COMPARE_PROCESSES = 8
 
 
-class CompareResult:
+class CompareResult(object):
     def __init__(self):
         self.result = {
             settings.ImageErrorLevel.INVALID: 0,
@@ -105,13 +105,10 @@ def _compare_configs_internal(pathbuilder, configs):
     return total_diff / combo_count
 
 
-def compare(pb, cids=None, sizes=settings.DEFAULT.tagsizes,
+def compare(pb, cids, sizes=settings.DEFAULT.tagsizes,
             types=settings.DEFAULT.tagtypes,
-            comparison="latest",    # TODO: Don't hardcode
+            comparison="latest",  # TODO: Don't hardcode
             configs=None):
-    if not cids:
-        cids = placelocal.get_cids(cids=settings.DEFAULT.campaigns,
-                                   pids=settings.DEFAULT.publishers)
     if not configs:
         configs = settings.DEFAULT.all_comparisons[comparison]
 
@@ -153,13 +150,17 @@ def __handle_output(pathbuilder, result_image, diff, prefix=""):
 
 
 def main(build=None):
+    LOGGER.info("Starting compare for cid=%s, pids=%s...",
+                settings.DEFAULT.campaigns, settings.DEFAULT.publishers)
     output.aggregate()
 
     if not build:
         build = output.generate_build_string()
     jobname = "compare_" + build
     pb = output.create(build=jobname)
-    compare(pb)
+
+    cids = placelocal.get_cids_from_settings()
+    compare(pb, cids=cids)
     return pb
 
 

tagcompare/logger.py

@@ -4,6 +4,10 @@
 import settings
 
 
+def set_level_from_settings():
+    logging.getLogger("tagcompare").setLevel(settings.DEFAULT.loglevel)
+
+
 class Logger(object):
     def __init__(self, name, directory=None, writefile=False):
         if directory and not writefile:
@@ -22,7 +26,7 @@ def __init__(self, name, directory=None, writefile=False):
         logger = logging.getLogger('tagcompare.%s' % name)
 
         # Set default log levels
-        logging.getLogger("tagcompare").setLevel(logging.DEBUG)
+        logging.getLogger("tagcompare").setLevel(settings.DEFAULT.loglevel)
         if not logger.handlers:
             if writefile:
                 logger.addHandler(self.__file_handler())
@@ -52,7 +56,7 @@ def __stream_handler(self):
         handler = logging.StreamHandler()
         formatter = logging.Formatter('%(levelname)s: %(message)s')
         handler.setFormatter(formatter)
-        handler.setLevel(settings.LOG_LEVEL)
+        handler.setLevel(settings.DEFAULT.loglevel)
         return handler
 
     def get(self):

tagcompare/main.py

@@ -1,10 +1,113 @@
+import argparse
+import sys
+import logging
+
 import capture
 import compare
+import settings
+import logger
+
+
+LOGGER = logger.Logger(name="main", writefile=True).get()
+
+
+def __query_yes_no(question, default="yes"):
+    """Ask a yes/no question via raw_input() and return their answer.
+
+    "question" is a string that is presented to the user.
+    "default" is the presumed answer if the user just hits <Enter>.
+        It must be "yes" (the default), "no" or None (meaning
+        an answer is required of the user).
+
+    The "answer" return value is True for "yes" or False for "no".
+
+    Source: http://code.activestate.com/recipes/577058/
+    """
+    valid = {"yes": True, "y": True, "ye": True,
+             "no": False, "n": False}
+    if default is None:
+        prompt = " [y/n] "
+    elif default == "yes":
+        prompt = " [Y/n] "
+    elif default == "no":
+        prompt = " [y/N] "
+    else:
+        raise ValueError("invalid default answer: '%s'" % default)
+
+    while True:
+        sys.stdout.write(question + prompt)
+        choice = raw_input().lower()
+        if default is not None and choice == '':
+            return valid[default]
+        elif choice in valid:
+            return valid[choice]
+        else:
+            sys.stdout.write("Please respond with 'yes' or 'no' "
+                             "(or 'y' or 'n').\n")
+
+
+def __parse_params_to_settings():
+    """
+    Handles parsing commandline args and set appropriate settings from them
+    :return: the key/value pair for the commandline args parsed
+    """
+    parser = argparse.ArgumentParser()
+    parser.add_argument('--capture-only', action='store_true', default=False,
+                        help='Run capture only without compare')
+    parser.add_argument('-d', '--domain',
+                        default=None,
+                        help='Domain, i.e. www.placelocal.com')
+
+    parser.add_argument('--verbose', action='store_true', default=False,
+                        help='Enable verbose logging for debugging')
+    parser.add_argument('--version', action='store_true', default=False,
+                        help='Show version')
+
+    # Either campaigns or publishers need to be specified, but not both
+    group = parser.add_mutually_exclusive_group(required=False)
+    group.add_argument('-c', '--campaigns', nargs='+',
+                       help='Space separated list of campaign ids')
+    group.add_argument('-p', '--publishers', nargs='+',
+                       help='Space separated list of publisher ids')
+
+    args = parser.parse_args()
+    LOGGER.debug("tagcompare params: %s", args)
+    return args
+
+
+def __update_settings_from_args(args):
+    # Update settings based on params
+    if args.campaigns or args.publishers:
+        settings.DEFAULT.campaigns = args.campaigns
+        settings.DEFAULT.publishers = args.publishers
+
+    if args.domain:
+        settings.DEFAULT.domain = args.domain
+
+    if args.verbose:
+        settings.DEFAULT.loglevel = logging.DEBUG
 
 
 def main():
+    args = __parse_params_to_settings()
+
+    if args.version:
+        # This gets handled by setup.py, we just need to not run the main routine
+        exit(0)
+
+    __update_settings_from_args(args)
+    proceed = __query_yes_no("Start tagcompare against {} for cid={}, pids={}?"
+                             .format(settings.DEFAULT.domain,
+                                     settings.DEFAULT.campaigns,
+                                     settings.DEFAULT.publishers))
+
+    if not proceed:
+        print("Stopping tagcompare on user input")
+        exit(0)
+
     jobname = capture.main()
-    compare.main(build=jobname)
+    if not args.capture_only:
+        compare.main(build=jobname)
 
 
 if __name__ == '__main__':

tagcompare/placelocal.py

@@ -10,11 +10,13 @@
 
 
 TIMESTAMP = time.strftime("%Y%m%d-%H%M%S")
-PL_DOMAIN = settings.DEFAULT.domain
 LOGGER = logger.Logger(__name__).get()
 
 
-def __get_route(route, domain=PL_DOMAIN):
+def __get_route(route, domain=None):
+    if not domain:
+        domain = settings.DEFAULT.domain
+
     url = str.format("https://{}/{}", domain, route)
     r = requests.get(url, headers=settings.DEFAULT.get_placelocal_headers())
     if r.status_code != 200:
@@ -61,9 +63,6 @@ def __get_tags(cid):
         return None
 
     tags_data = data['http_ad_tags']
-    if not isinstance(tags_data, dict):
-        raise ValueError("tag_data is not a dict!:\n %s", tags_data)
-    # LOGGER.debug("__get_tags result:\n%s\n\n\n", tags_data)
     return tags_data
 
 
@@ -76,8 +75,8 @@ def get_tags_for_campaigns(cids):
     """
     if not cids:
         raise ValueError("cids not defined!")
-    LOGGER.info(
-        "get tags for %s campaigns: %s...", len(cids), cids)
+    LOGGER.debug(
+        "Get tags for %s campaigns: %s...", len(cids), cids)
 
     tp = ThreadPool(processes=10)
     results = {}
@@ -128,20 +127,26 @@ def _get_all_pids(pids):
     return result
 
 
-def get_cids(cids=None, pids=None):
+def get_cids_from_settings(settings_obj=settings.DEFAULT):
+    cids = settings_obj.campaigns
+    pids = settings_obj.publishers
+    return _get_cids(cids, pids)
+
+
+def _get_cids(cids=None, pids=None):
     """
     Gets a list of campaign ids from a combinination of cids and pids
     :param cids: campaign ids, directly gets appended to the list of cids
     :param pids: publisher or superpub ids, this is confusing - I know
     :return: a list of campaign ids
     """
+    if cids:
+        return cids
+
     if not pids:
-        if cids:
-            return cids
-        raise ValueError("pid must be specified if there are no cids!")
+        raise ValueError("pids must be specified if there are no cids!")
 
-    if pids and not cids:
-        cids = []
+    cids = []
     all_pids = _get_all_pids(pids)
     for pid in all_pids:
         new_cids = __get_active_campaigns(pid)