Merge pull request #430 from zalando-stups/pylint-friday-2017-01

Fix some issues found by pylint

senza/__init__.py

@@ -1 +1,6 @@
+"""
+Deploy immutable application stacks and create and execute AWS CloudFormation
+templates in a sane way
+"""
+
 __version__ = '0.90'

senza/arguments.py

@@ -1,10 +1,16 @@
+"""
+Functions and decorators related to command line arguments
+"""
+
+# invalid-name is disabled to match the style of other click options
+# pylint: disable=locally-disabled, invalid-name
 import boto3.session
 import click
 
 from .error_handling import HandleExceptions
 
 
-def validate_region(ctx, param, value):
+def validate_region(ctx, param, value):  # pylint: disable=locally-disabled, unused-argument
     """Validate Click region param parameter."""
 
     if value is not None:
@@ -19,7 +25,11 @@ def validate_region(ctx, param, value):
     return value
 
 
-def set_stacktrace_visible(ctx, param, value):
+def set_stacktrace_visible(ctx, param, value):  # pylint: disable=locally-disabled, unused-argument
+    """
+    Callback to define whether to display the stacktrace in case of an
+    unhandled error.
+    """
     HandleExceptions.stacktrace_visible = value
 
 

senza/configuration.py

@@ -1,3 +1,10 @@
+"""
+Class and instance to read and write senza configuration.
+
+Senza configuration consists of an hierarchical yaml file with
+sections > keys > values, which are represented in the form SECTION.KEY
+"""
+
 from collections.abc import MutableMapping
 from pathlib import Path
 from typing import Dict, Tuple
@@ -7,9 +14,16 @@
 
 from .exceptions import InvalidConfigKey
 
+CONFIGURATION_PATH = Path(get_app_dir('senza')) / "config.yaml"
+
 
 class Configuration(MutableMapping):
 
+    """
+    Class to read and write senza configuration as map. Keys take the form of
+    SECTION.KEY
+    """
+
     def __init__(self, path: Path):
         self.config_path = path
 
@@ -25,12 +39,12 @@ def __getitem__(self, key: str) -> str:
 
     def __setitem__(self, key: str, value):
         section, sub_key = self.__split_key(key)
-        configuration = self.raw_dict
+        cfg = self.raw_dict
 
-        if section not in configuration:
-            configuration[section] = {}
-        configuration[section][sub_key] = str(value)
-        self.__save(configuration)
+        if section not in self.raw_dict:
+            cfg[section] = {}
+        cfg[section][sub_key] = str(value)
+        self.__save(cfg)
 
     def __delitem__(self, key):
         section, sub_key = self.__split_key(key)
@@ -79,4 +93,4 @@ def raw_dict(self) -> Dict[str, Dict[str, str]]:
         return cfg
 
 
-configuration = Configuration(Path(get_app_dir('senza')) / "config.yaml")
+configuration = Configuration(CONFIGURATION_PATH)  # pylint: disable=locally-disabled, invalid-name

senza/error_handling.py

@@ -6,7 +6,7 @@
 import sys
 from tempfile import NamedTemporaryFile
 from traceback import format_exception
-from typing import Any, Dict, Optional  # noqa: F401
+from typing import Optional  # noqa: F401
 
 import senza
 import yaml.constructor
@@ -140,7 +140,7 @@ def __call__(self, *args, **kwargs):
             message = ("{}\nRun `senza init` to (re-)create "
                        "the security group.").format(error)
             die_fatal_error(message)
-        except Exception as unknown_exception:
+        except Exception as unknown_exception:  # pylint: disable=locally-disabled, broad-except
             # Catch All
             self.die_unknown_error(unknown_exception)
 
@@ -159,4 +159,4 @@ def setup_sentry(sentry_endpoint: Optional[str]):
     return sentry_client
 
 
-sentry = setup_sentry(configuration.get('sentry.endpoint'))
+sentry = setup_sentry(configuration.get('sentry.endpoint'))  # pylint: disable=locally-disabled, invalid-name

senza/templates/__init__.py

@@ -1,13 +1,22 @@
+"""
+Modules and functions for senza component templates
+"""
+
 from types import ModuleType, FunctionType
 import pkg_resources
 
 
-def get_template_description(name, module: ModuleType):
+def get_template_description(name, module: ModuleType) -> str:
+    """
+    Gets the human-readable template description based on the name of the
+    component and the component's module docstring
+    """
     return '{}: {}'.format(name, (module.__doc__ or "").strip())
 
 
 def has_functions(module, names):
-    return all(isinstance(getattr(module, function_name, None), FunctionType) for function_name in names)
+    return all(isinstance(getattr(module, function_name, None), FunctionType)
+               for function_name in names)
 
 
 def get_templates() -> dict:
@@ -16,15 +25,15 @@ def get_templates() -> dict:
     """
     entry_points = pkg_resources.iter_entry_points('senza.templates')
     template_modules = {}
-    for e in entry_points:  # type: pkg_resources.EntryPoint
+    for entry_point in entry_points:  # type: pkg_resources.EntryPoint
         try:
-            module = e.resolve()
+            module = entry_point.resolve()
         except ImportError:
             # ignore bad entry points
             continue
         else:
             # make sure the entry point resolves to a module with the essential interface functions
             if (isinstance(module, ModuleType) and
-               has_functions(module, ('gather_user_variables', 'generate_definition'))):
-                template_modules[e.name] = module
+                    has_functions(module, ('gather_user_variables', 'generate_definition'))):
+                template_modules[entry_point.name] = module
     return template_modules

senza/templates/postgresapp.py

@@ -398,7 +398,7 @@ def gather_user_variables(variables, region, account_info):
     prompt(variables, 'instance_type', 'EC2 instance type', default='t2.medium')
 
     variables['hosted_zone'] = account_info.Domain or defaults['hosted_zone']
-    if (variables['hosted_zone'][-1:] != '.'):
+    if variables['hosted_zone'][-1:] != '.':
         variables['hosted_zone'] += '.'
     prompt(variables, 'discovery_domain', 'ETCD Discovery Domain',
            default='postgres.' + variables['hosted_zone'][:-1])
@@ -503,11 +503,14 @@ def get_latest_image(registry_domain='registry.opensource.zalan.do', team='acid'
     Gets the full name of latest image for an artifact
     """
     try:
-        r = requests.get('https://{0}/teams/{1}/artifacts/{2}/tags'.format(registry_domain, team, artifact))
-        if r.ok:
+        url = 'https://{0}/teams/{1}/artifacts/{2}/tags'.format(registry_domain,
+                                                                team,
+                                                                artifact)
+        response = requests.get(url)
+        if response.ok:
             # sort the tags by creation date
             latest = None
-            for entry in sorted(r.json(), key=lambda t: t['created'], reverse=True):
+            for entry in sorted(response.json(), key=lambda t: t['created'], reverse=True):
                 tag = entry['name']
                 # try to avoid snapshots if possible
                 if 'SNAPSHOT' not in tag:

senza/templates/redisnode.py

@@ -1,6 +1,6 @@
-'''
+"""
 Elasticache node running redis, without replication / HA (for caching)
-'''
+"""
 
 from clickclick import warning
 from senza.utils import pystache_render
@@ -28,7 +28,10 @@
 '''
 
 
-def gather_user_variables(variables, region, account_info):
+def gather_user_variables(variables, region, account_info):  # pylint: disable=locally-disabled, unused-argument
+    """
+    Gather all the variables needed to create the redis node
+    """
     # maximal 32 characters because of the loadbalancer-name
     prompt(variables, 'application_id', 'Application ID', default='hello-world',
            value_proc=check_value(18, '^[a-zA-Z][-a-zA-Z0-9]*$'))
@@ -38,12 +41,15 @@ def gather_user_variables(variables, region, account_info):
 
     rules_missing = check_security_group(sg_name, [('tcp', 6379)], region, allow_from_self=True)
     if ('tcp', 6379) in rules_missing:
-        warning('Security group {} does not allow tcp/6379 access, you will not be able to access your redis'.format(
-            sg_name))
+        warning('Security group {} does not allow tcp/6379 access, '
+                'you will not be able to access your redis'.format(sg_name))
 
     return variables
 
 
 def generate_definition(variables):
+    """
+    Generates the redis node definition yaml from template
+    """
     definition_yaml = pystache_render(TEMPLATE, variables)
     return definition_yaml

senza/traffic.py

@@ -1,3 +1,7 @@
+"""
+Functions related to Traffic management
+"""
+
 import collections
 from json import JSONEncoder
 from typing import Dict, Iterator
@@ -63,6 +67,9 @@ def get_weights(dns_names: list, identifier: str, all_identifiers) -> ({str: int
 
 
 def calculate_new_weights(delta, identifier, known_record_weights, percentage):
+    """
+    Calculates the new weights for the all the Route53 records
+    """
     new_record_weights = {}
     deltas = {}
     for i, w in known_record_weights.items():
@@ -88,8 +95,9 @@ def calculate_new_weights(delta, identifier, known_record_weights, percentage):
 def compensate(calculation_error, compensations, identifier, new_record_weights, partial_count,
                percentage, identifier_versions):
     """
-    Compensate for the rounding errors as well as for the fact, that we do not allow to bring down the minimal weights
-    lower then minimal possible value not to disable traffic from the minimally configured versions (1) and
+    Compensate for the rounding errors as well as for the fact, that we do not
+    allow to bring down the minimal weights lower then minimal possible value
+    not to disable traffic from the minimally configured versions (1) and
     we do not allow to add any values to the already disabled versions (0).
     """
     # distribute the error on the versions, other then the current one
@@ -103,11 +111,11 @@ def compensate(calculation_error, compensations, identifier, new_record_weights,
     for i in sorted(new_record_weights.keys(), key=lambda x: identifier_versions[x], reverse=True):
         if i == identifier:
             continue
-        nw = new_record_weights[i] + part
-        if nw <= 0:
+        new_weight = new_record_weights[i] + part
+        if new_weight <= 0:
             # do not remove the traffic from the minimal traffic versions
             continue
-        new_record_weights[i] = nw
+        new_record_weights[i] = new_weight
         calculation_error -= part
         compensations[i] = part
         if calculation_error == 0:
@@ -119,7 +127,7 @@ def compensate(calculation_error, compensations, identifier, new_record_weights,
         warning(
             ("Changing given percentage from {} to {} " +
              "because all other versions are already getting the possible minimum traffic").format(
-                percentage / PERCENT_RESOLUTION, adjusted_percentage / PERCENT_RESOLUTION))
+                 percentage / PERCENT_RESOLUTION, adjusted_percentage / PERCENT_RESOLUTION))
         percentage = adjusted_percentage
         new_record_weights[identifier] = percentage
     assert calculation_error == 0
@@ -200,8 +208,7 @@ def dump_traffic_changes(stack_name: str,
                          known_record_weights: {str: int},
                          new_record_weights: {str: int},
                          compensations: {str: int},
-                         deltas: {str: int}
-                         ):
+                         deltas: {str: int}):
     """
     dump changes to the traffic settings for the given versions
     """
@@ -236,10 +243,12 @@ def dump_traffic_changes(stack_name: str,
 
 
 def print_traffic_changes(message: list):
-    print_table('stack_name version identifier old_weight% delta compensation new_weight% current'.split(), message)
+    print_table(['stack_name', 'version', 'identifier', 'old_weight%',
+                 'delta', 'compensation', 'new_weight%', 'current'], message)
 
 
-class StackVersion(collections.namedtuple('StackVersion', 'name version domain lb_dns_name notification_arns')):
+class StackVersion(collections.namedtuple('StackVersion',
+                                          'name version domain lb_dns_name notification_arns')):
     @property
     def identifier(self):
         return '{}-{}'.format(self.name, self.version)
@@ -271,7 +280,9 @@ def get_stack_versions(stack_name: str, region: str) -> Iterator[StackVersion]:
             elif res.resource_type == 'AWS::Route53::RecordSet':
                 if 'version' not in res.logical_id.lower():
                     domain.append(res.physical_resource_id)
-        yield StackVersion(stack_name, get_tag(details.tags, 'StackVersion'), domain, lb_dns_name, notification_arns)
+        yield StackVersion(stack_name,
+                           get_tag(details.tags, 'StackVersion'),
+                           domain, lb_dns_name, notification_arns)
 
 
 def get_version(versions: list, version: str):
@@ -291,8 +302,7 @@ def get_records(domain: str):
         while result['IsTruncated']:
             recordfilter = {'HostedZoneId': hosted_zone.id,
                             'StartRecordName': result['NextRecordName'],
-                            'StartRecordType': result['NextRecordType']
-                            }
+                            'StartRecordType': result['NextRecordType']}
             if result.get('NextRecordIdentifier'):
                 recordfilter['StartRecordIdentifier'] = result.get('NextRecordIdentifier')
 
@@ -314,10 +324,13 @@ def print_version_traffic(stack_ref: StackReference, region):
         raise click.UsageError('No stack version of "{}" found'.format(stack_ref.name))
 
     if not version.domain:
-        raise click.UsageError('Stack {} version {} has no domain'.format(version.name, version.version))
+        raise click.UsageError('Stack {} version {} has '
+                               'no domain'.format(version.name,
+                                                  version.version))
 
-    known_record_weights, partial_count, partial_sum = get_weights(version.dns_name, version.identifier,
-                                                                   identifier_versions.keys())
+    known_record_weights, _, _ = get_weights(version.dns_name,
+                                             version.identifier,
+                                             identifier_versions.keys())
 
     rows = [
         {
@@ -344,16 +357,18 @@ def change_version_traffic(stack_ref: StackReference, percentage: float,
                            region: str):
     versions = list(get_stack_versions(stack_ref.name, region))
     arns = []
-    for v in versions:
-        arns = arns + v.notification_arns
+    for each_version in versions:
+        arns = arns + each_version.notification_arns
     identifier_versions = collections.OrderedDict(
         (version.identifier, version.version) for version in versions)
     version = get_version(versions, stack_ref.version)
 
     identifier = version.identifier
 
     if not version.domain:
-        raise click.UsageError('Stack {} version {} has no domain'.format(version.name, version.version))
+        raise click.UsageError('Stack {} version {} has '
+                               'no domain'.format(version.name,
+                                                  version.version))
 
     percentage = int(percentage * PERCENT_RESOLUTION)
     known_record_weights, partial_count, partial_sum = get_weights(version.dns_name, identifier,
@@ -362,7 +377,8 @@ def change_version_traffic(stack_ref: StackReference, percentage: float,
     if partial_count == 0 and percentage == 0:
         # disable the last remaining version
         new_record_weights = {i: 0 for i in known_record_weights.keys()}
-        message = 'DNS record "{dns_name}" will be removed from that stack'.format(dns_name=version.dns_name)
+        message = ('DNS record "{dns_name}" will be removed from that '
+                   'stack'.format(dns_name=version.dns_name))
         ok(msg=message)
     else:
         with Action('Calculating new weights..'):
@@ -375,7 +391,10 @@ def change_version_traffic(stack_ref: StackReference, percentage: float,
                     # will put the only last version to full traffic percentage
                     compensations[identifier] = FULL_PERCENTAGE - percentage
                     percentage = int(FULL_PERCENTAGE)
-            new_record_weights, deltas = calculate_new_weights(delta, identifier, known_record_weights, percentage)
+            new_record_weights, deltas = calculate_new_weights(delta,
+                                                               identifier,
+                                                               known_record_weights,
+                                                               percentage)
             total_weight = sum(new_record_weights.values())
             calculation_error = FULL_PERCENTAGE - total_weight
             if calculation_error and calculation_error < FULL_PERCENTAGE:
@@ -401,7 +420,9 @@ def inform_sns(arns: list, message: str, region):
     sns_topics = set(arns)
     sns = BotoClientProxy('sns', region_name=region)
     for sns_topic in sns_topics:
-        sns.publish(TopicArn=sns_topic, Subject="SenzaTrafficRedirect", Message=jsonizer.encode((message)))
+        sns.publish(TopicArn=sns_topic,
+                    Subject="SenzaTrafficRedirect",
+                    Message=jsonizer.encode(message))
 
 
 def resolve_to_ip_addresses(dns_name: str) -> set: