refactor: add support for Python 3 (#490) (#491)

brettstack · web-flow · commit e8f74f56d19a · 2018-06-28T15:29:41.000-07:00
* feat: add support for Python 3 (#428) * refactor: Updated imports to by Py3 compliant * refactor: Move class variable creation to constructor in globals.py Without moving this to the __init__, the globals.py file will not run in Py3 because it can't reference the constants. * refactor: Update update_policy.py to be Py3 compliant In Py3, the function .iteritems() on a dict was removed and replaced with .items(). * refactor: Update deployment_preference_collection.py to be Py3 compliant In Py3, .itervalues() and .iteritems() was replaced with .values() and .items(), respectfully. * refactor: Update swagger.py to be Py3 compliant In Py3, the .keys() method on a dictionary returns a dict_keys object that is a view into the dictionary. In Py2, it returns a list. To support Py3, we need to convert the .keys() to a list. * refactor: Update intrinsics.py to be Py3 compliant In Py3, the .keys() method on a dictionary returns a dict_keys object that is a view into the dictionary. In Py2, it returns a list. To support Py3, we need to convert the .keys() to a list. * Staging translator.py changes Updated .iteritems() to items() * refactor: More updating to be Py3 compliant * refactor: Make hashing constisent between py2 and py3 * refactor: Make exceptions sortable to allow error case tests to pass in Py3 * fix: add support for Python 3 (#445) * refactor: Updated imports to by Py3 compliant * refactor: Move class variable creation to constructor in globals.py Without moving this to the __init__, the globals.py file will not run in Py3 because it can't reference the constants. * refactor: Update update_policy.py to be Py3 compliant In Py3, the function .iteritems() on a dict was removed and replaced with .items(). * refactor: Update deployment_preference_collection.py to be Py3 compliant In Py3, .itervalues() and .iteritems() was replaced with .values() and .items(), respectfully. * refactor: Update swagger.py to be Py3 compliant In Py3, the .keys() method on a dictionary returns a dict_keys object that is a view into the dictionary. In Py2, it returns a list. To support Py3, we need to convert the .keys() to a list. * refactor: Update intrinsics.py to be Py3 compliant In Py3, the .keys() method on a dictionary returns a dict_keys object that is a view into the dictionary. In Py2, it returns a list. To support Py3, we need to convert the .keys() to a list. * Staging translator.py changes Updated .iteritems() to items() * refactor: More updating to be Py3 compliant * refactor: Make hashing constisent between py2 and py3 * refactor: Make exceptions sortable to allow error case tests to pass in Py3 * feat: Run tox from Travis-CI * feat: Update tox to run in Py2 and Py3 * refactor: Force sorting behavior to be Py2 compatible and update Deployment logicalid hash * fix: Update tox to run tests against Py27 and Py36 * Update Travis config to run Py2 and Py3 tests in parallel * Setting region env var in tox file for Travis to pick up * Set AWS region in travis file * Pass AWS_* env vars to tox * Fixing ordering of resource types in Globals error message * Py2/3 compatible implementation of string encoding for logicalId generator Also added lots of comments explaining why/how the deep sorting of lists work in unit tests * Removing redundant usage of bytes
diff --git a/.travis.yml b/.travis.yml
@@ -2,6 +2,16 @@
 sudo: false
 language: python
 
+matrix:
+  include:
+    - python: 3.6
+      env:
+        - TOXENV=py36
+    - python: 2.7
+      env:
+        - TOXENV=py27
+
+
 install:
 # Install the code requirements
 - make init
@@ -10,7 +20,7 @@ install:
 
 script:
 # Runs unit tests
-- make pr
+- tox
 
 # Build docs pages only from master branch
 - if [ "$TRAVIS_BRANCH" = "master" ]; then make build-docs; fi
diff --git a/samtranslator/intrinsics/resolver.py b/samtranslator/intrinsics/resolver.py
@@ -117,7 +117,7 @@ def _traverse_dict(self, input_dict, resolution_data, resolver_method):
         :param resolver_method: Method that can actually resolve an intrinsic function, if it detects one
         :return: Modified dictionary with values resolved
         """
-        for key, value in input_dict.iteritems():
+        for key, value in input_dict.items():
             input_dict[key] = self._traverse(value, resolution_data, resolver_method)
 
         return input_dict
@@ -150,7 +150,7 @@ def _try_resolve_parameter_refs(self, input, parameters):
         if not self._is_intrinsic_dict(input):
             return input
 
-        function_type = input.keys()[0]
+        function_type = list(input.keys())[0]
         return self.supported_intrinsics[function_type].resolve_parameter_refs(input, parameters)
 
     def _try_resolve_sam_resource_refs(self, input, supported_resource_refs):
@@ -168,7 +168,7 @@ def _try_resolve_sam_resource_refs(self, input, supported_resource_refs):
         if not self._is_intrinsic_dict(input):
             return input
 
-        function_type = input.keys()[0]
+        function_type = list(input.keys())[0]
         return self.supported_intrinsics[function_type].resolve_resource_refs(input, supported_resource_refs)
 
     def _is_intrinsic_dict(self, input):
@@ -181,4 +181,4 @@ def _is_intrinsic_dict(self, input):
         # All intrinsic functions are dictionaries with just one key
         return isinstance(input, dict) \
                and len(input) == 1 \
-               and input.keys()[0] in self.supported_intrinsics
+               and list(input.keys())[0] in self.supported_intrinsics
diff --git a/samtranslator/model/__init__.py b/samtranslator/model/__init__.py
@@ -68,7 +68,7 @@ def __init__(self, logical_id, relative_id=None, depends_on=None, attributes=Non
 
         self.resource_attributes = {}
         if attributes is not None:
-            for attr, value in attributes.iteritems():
+            for attr, value in attributes.items():
                 self.set_resource_attribute(attr, value)
 
     @classmethod
@@ -367,7 +367,7 @@ def get_resource_references(self, generated_cfn_resources, supported_resource_re
         # Create a map of {ResourceType: LogicalId} for quick access
         resource_id_by_type = {resource.resource_type:resource.logical_id for resource in generated_cfn_resources}
 
-        for property, cfn_type in self.referable_properties.iteritems():
+        for property, cfn_type in self.referable_properties.items():
             if cfn_type in resource_id_by_type:
                 supported_resource_refs.add(self.logical_id, property, resource_id_by_type[cfn_type])
 
diff --git a/samtranslator/model/eventsources/cloudwatchlogs.py b/samtranslator/model/eventsources/cloudwatchlogs.py
@@ -1,9 +1,9 @@
 from samtranslator.model import PropertyType
-from samtranslator.model.types import is_str
-from samtranslator.translator.arn_generator import ArnGenerator
 from samtranslator.model.intrinsics import fnSub
 from samtranslator.model.log import SubscriptionFilter
-from push import PushEventSource
+from samtranslator.model.types import is_str
+from samtranslator.translator.arn_generator import ArnGenerator
+from .push import PushEventSource
 
 class CloudWatchLogs(PushEventSource):
     """CloudWatch Logs event source for SAM Functions."""
diff --git a/samtranslator/model/exceptions.py b/samtranslator/model/exceptions.py
@@ -6,7 +6,7 @@ class InvalidDocumentException(Exception):
         causes -- list of errors which caused this document to be invalid
     """
     def __init__(self, causes):
-        self._causes = causes
+        self._causes = sorted(causes)
 
     @property
     def message(self):
@@ -61,6 +61,9 @@ def __init__(self, logical_id,  message):
         self._logical_id = logical_id
         self._message = message
 
+    def __lt__(self, other):
+        return self._logical_id < other._logical_id
+
     @property
     def message(self):
         return 'Resource with id [{}] is invalid. {}'.format(self._logical_id, self._message)
diff --git a/samtranslator/model/function_policies.py b/samtranslator/model/function_policies.py
@@ -140,7 +140,7 @@ def _is_policy_template(self, policy):
         return self._policy_template_processor is not None and \
                 isinstance(policy, dict) and \
                 len(policy) == 1 and \
-                self._policy_template_processor.has(policy.keys()[0]) is True
+                self._policy_template_processor.has(list(policy.keys())[0]) is True
 
 
 
diff --git a/samtranslator/model/intrinsics.py b/samtranslator/model/intrinsics.py
@@ -51,7 +51,7 @@ def is_instrinsic(input):
         and isinstance(input, dict) \
         and len(input) == 1:
 
-        key = input.keys()[0]
+        key = list(input.keys())[0]
         return key == "Ref" or key == "Condition" or key.startswith("Fn::")
 
     return False
diff --git a/samtranslator/model/preferences/deployment_preference_collection.py b/samtranslator/model/preferences/deployment_preference_collection.py
@@ -1,9 +1,9 @@
-from deployment_preference import DeploymentPreference
+from .deployment_preference import DeploymentPreference
 from samtranslator.model.codedeploy import CodeDeployApplication
 from samtranslator.model.codedeploy import CodeDeployDeploymentGroup
 from samtranslator.model.iam import IAMRole
-from samtranslator.model.update_policy import UpdatePolicy
 from samtranslator.model.intrinsics import fnSub
+from samtranslator.model.update_policy import UpdatePolicy
 from samtranslator.translator.arn_generator import ArnGenerator
 
 CODE_DEPLOY_SERVICE_ROLE_LOGICAL_ID = 'CodeDeployServiceRole'
@@ -53,22 +53,22 @@ def any_enabled(self):
         """
         :return: boolean whether any deployment preferences in the collection are enabled
         """
-        return any(preference.enabled for preference in self._resource_preferences.itervalues())
+        return any(preference.enabled for preference in self._resource_preferences.values())
 
     def can_skip_service_role(self):
         """
         If every one of the deployment preferences have a custom IAM role provided, we can skip creating the
         service role altogether.
         :return: True, if we can skip creating service role. False otherwise
         """
-        return all(preference.role for preference in self._resource_preferences.itervalues())
+        return all(preference.role for preference in self._resource_preferences.values())
 
 
     def enabled_logical_ids(self):
         """
         :return: only the logical id's for the deployment preferences in this collection which are enabled
         """
-        return [logical_id for logical_id, preference in self._resource_preferences.iteritems() if preference.enabled]
+        return [logical_id for logical_id, preference in self._resource_preferences.items() if preference.enabled]
 
     def _codedeploy_application(self):
         codedeploy_application_resource = CodeDeployApplication(CODEDEPLOY_APPLICATION_LOGICAL_ID)
diff --git a/samtranslator/model/s3_utils/uri_parser.py b/samtranslator/model/s3_utils/uri_parser.py
@@ -1,6 +1,5 @@
-from urlparse import urlparse, parse_qs
-
 from six import string_types
+from six.moves.urllib.parse import urlparse, parse_qs
 
 
 def parse_s3_uri(uri):
diff --git a/samtranslator/model/sam_resources.py b/samtranslator/model/sam_resources.py
@@ -1,24 +1,25 @@
 """ SAM macro definitions """
 from six import string_types
-from tags.resource_tagging import get_tag_list
+
 import samtranslator.model.eventsources
 import samtranslator.model.eventsources.pull
 import samtranslator.model.eventsources.push
 import samtranslator.model.eventsources.cloudwatchlogs
+from .api.api_generator import ApiGenerator
+from .s3_utils.uri_parser import parse_s3_uri
+from .tags.resource_tagging import get_tag_list
 from samtranslator.model import (PropertyType, SamResourceMacro,
                                  ResourceTypeResolver)
+from samtranslator.model.apigateway import ApiGatewayDeployment, ApiGatewayStage
 from samtranslator.model.dynamodb import DynamoDBTable
 from samtranslator.model.exceptions import (InvalidEventException,
                                             InvalidResourceException)
+from samtranslator.model.function_policies import FunctionPolicies, PolicyTypes
 from samtranslator.model.iam import IAMRole, IAMRolePolicies
 from samtranslator.model.lambda_ import LambdaFunction, LambdaVersion, LambdaAlias
-from samtranslator.model.apigateway import ApiGatewayDeployment, ApiGatewayStage
 from samtranslator.model.types import dict_of, is_str, is_type, list_of, one_of, any_type
-from samtranslator.model.function_policies import FunctionPolicies, PolicyTypes
 from samtranslator.translator import logical_id_generator
 from samtranslator.translator.arn_generator import ArnGenerator
-from api.api_generator import ApiGenerator
-from s3_utils.uri_parser import parse_s3_uri
 
 
 class SamFunction(SamResourceMacro):
diff --git a/samtranslator/model/update_policy.py b/samtranslator/model/update_policy.py
@@ -25,6 +25,6 @@ def to_dict(self):
         :return: a dict that can be used as part of a cloudformation template
         """
         dict_with_nones = self._asdict()
-        codedeploy_lambda_alias_update_dict = dict((k, v) for k, v in dict_with_nones.iteritems()
+        codedeploy_lambda_alias_update_dict = dict((k, v) for k, v in dict_with_nones.items()
                                                    if v != ref(None) and v is not None)
         return {'CodeDeployLambdaAliasUpdate': codedeploy_lambda_alias_update_dict}
diff --git a/samtranslator/plugins/api/implicit_api_plugin.py b/samtranslator/plugins/api/implicit_api_plugin.py
@@ -100,7 +100,7 @@ def _get_api_events(self, function):
             return {}
 
         api_events = {}
-        for event_id, event in function.properties["Events"].iteritems():
+        for event_id, event in function.properties["Events"].items():
 
             if event and isinstance(event, dict) and event.get("Type") == "Api":
                 api_events[event_id] = event
@@ -117,7 +117,7 @@ def _process_api_events(self, function, api_events, template):
         :param SamTemplate template: SAM Template where Serverless::Api resources can be found
         """
 
-        for logicalId, event in api_events.iteritems():
+        for logicalId, event in api_events.items():
 
             event_properties = event.get("Properties", {})
             if not event_properties:
diff --git a/samtranslator/plugins/globals/globals.py b/samtranslator/plugins/globals/globals.py
@@ -52,14 +52,16 @@ class Globals(object):
         ]
     }
 
-    supported_resource_section_names = [x.replace(_RESOURCE_PREFIX, "") for x in supported_properties.keys()]
-
     def __init__(self, template):
         """
         Constructs an instance of this object
 
         :param dict template: SAM template to be parsed
         """
+        self.supported_resource_section_names = [x.replace(self._RESOURCE_PREFIX, "") for x in self.supported_properties.keys()]
+        # Sort the names for stability in list ordering
+        self.supported_resource_section_names.sort()
+
         self.template_globals = {}
 
         if self._KEYWORD in template:
@@ -109,7 +111,7 @@ def _parse(self, globals_dict):
         if not isinstance(globals_dict, dict):
             raise InvalidGlobalsSectionException(self._KEYWORD, "It must be a non-empty dictionary".format(self._KEYWORD))
 
-        for section_name, properties in globals_dict.iteritems():
+        for section_name, properties in globals_dict.items():
             resource_type = self._make_resource_type(section_name)
 
             if resource_type not in self.supported_properties:
@@ -122,7 +124,7 @@ def _parse(self, globals_dict):
             if not isinstance(properties, dict):
                 raise InvalidGlobalsSectionException(self._KEYWORD, "Value of ${section} must be a dictionary")
 
-            for key, value in properties.iteritems():
+            for key, value in properties.items():
                 supported = self.supported_properties[resource_type]
                 if key not in supported:
                     raise InvalidGlobalsSectionException(self._KEYWORD,
diff --git a/samtranslator/plugins/policies/policy_templates_plugin.py b/samtranslator/plugins/policies/policy_templates_plugin.py
@@ -57,8 +57,8 @@ def on_before_transform_resource(self, logical_id, resource_type, resource_prope
             # We are processing policy templates. We know they have a particular structure:
             # {"templateName": { parameter_values_dict }}
             template_data = policy_entry.data
-            template_name = template_data.keys()[0]
-            template_parameters = template_data.values()[0]
+            template_name = list(template_data.keys())[0]
+            template_parameters = list(template_data.values())[0]
 
             try:
 
@@ -67,7 +67,7 @@ def on_before_transform_resource(self, logical_id, resource_type, resource_prope
 
             except InsufficientParameterValues as ex:
                 # Exception's message will give lot of specific details
-                raise InvalidResourceException(logical_id, ex.message)
+                raise InvalidResourceException(logical_id, str(ex))
             except InvalidParameterValues:
                 raise InvalidResourceException(logical_id,
                                                "Must specify valid parameter values for policy template '{}'"
diff --git a/samtranslator/policy_template_processor/processor.py b/samtranslator/policy_template_processor/processor.py
@@ -58,7 +58,7 @@ def __init__(self, policy_templates_dict, schema=None):
         PolicyTemplatesProcessor._is_valid_templates_dict(policy_templates_dict, schema)
 
         self.policy_templates = {}
-        for template_name, template_value_dict in policy_templates_dict["Templates"].iteritems():
+        for template_name, template_value_dict in policy_templates_dict["Templates"].items():
             self.policy_templates[template_name] = Template.from_dict(template_name, template_value_dict)
 
     def has(self, template_name):
diff --git a/samtranslator/policy_template_processor/template.py b/samtranslator/policy_template_processor/template.py
@@ -46,7 +46,7 @@ def to_statement(self, parameter_values):
         # Select only necessary parameter_values. this is to prevent malicious or accidental
         # injection of values for parameters not intended in the template. This is important because "Ref" resolution
         # will substitute any references for which a value is provided.
-        necessary_parameter_values = {name: value for name, value in parameter_values.iteritems()
+        necessary_parameter_values = {name: value for name, value in parameter_values.items()
                                       if name in self.parameters}
 
         # Only "Ref" is supported
@@ -71,7 +71,7 @@ def missing_parameter_values(self, parameter_values):
         if not self._is_valid_parameter_values(parameter_values):
             raise InvalidParameterValues("Parameter values are required to process a policy template")
 
-        return list(self.parameters.viewkeys() - parameter_values.viewkeys())
+        return list(set(self.parameters.keys()) - set(parameter_values.keys()))
 
     @staticmethod
     def _is_valid_parameter_values(parameter_values):
diff --git a/samtranslator/region_configuration.py b/samtranslator/region_configuration.py
@@ -1,4 +1,5 @@
-from translator.arn_generator import ArnGenerator
+from .translator.arn_generator import ArnGenerator
+
 
 class RegionConfiguration(object):
     """
diff --git a/samtranslator/sdk/template.py b/samtranslator/sdk/template.py
@@ -26,7 +26,7 @@ def iterate(self, resource_type=None):
         :yields (string, SamResource): Tuple containing LogicalId and the resource
         """
 
-        for logicalId, resource_dict in self.resources.iteritems():
+        for logicalId, resource_dict in self.resources.items():
 
             resource = SamResource(resource_dict)
             needs_filter = resource.valid()
diff --git a/samtranslator/swagger/swagger.py b/samtranslator/swagger/swagger.py
@@ -262,7 +262,7 @@ def _make_cors_allowed_methods_for_path(self, path):
             return ""
 
         # At this point, value of Swagger path should be a dictionary with method names being the keys
-        methods = self.paths[path].keys()
+        methods = list(self.paths[path].keys())
 
         if self._X_ANY_METHOD in methods:
             # API Gateway's ANY method is not a real HTTP method but a wildcard representing all HTTP methods
diff --git a/samtranslator/translator/logical_id_generator.py b/samtranslator/translator/logical_id_generator.py
@@ -1,7 +1,9 @@
 import hashlib
 import json
+import sys
 from six import string_types
 
+
 class LogicalIdGenerator(object):
 
     # NOTE: Changing the length of the hash will change backwards compatibility. This will break the stability contract
@@ -54,8 +56,19 @@ def get_hash(self, length=HASH_LENGTH):
         """
 
         data_hash = ""
-        if self.data_str:
-            data_hash = hashlib.sha1(bytes(self.data_str)).hexdigest()
+        if not self.data_str:
+            return data_hash
+
+        encoded_data_str = self.data_str
+        if sys.version_info.major == 2:
+            # In Py2, only unicode needs to be encoded.
+            if isinstance(self.data_str, unicode):
+                encoded_data_str = self.data_str.encode('utf-8')
+        else:
+            # data_str should always be unicode on python 3
+            encoded_data_str = self.data_str.encode('utf-8')
+
+        data_hash = hashlib.sha1(encoded_data_str).hexdigest()
 
         return data_hash[:length]
 
diff --git a/samtranslator/translator/translator.py b/samtranslator/translator/translator.py
@@ -179,7 +179,7 @@ def _add_default_parameter_values(self, sam_template, parameter_values):
             return parameter_values
 
         default_values = {}
-        for param_name, value in parameter_definition.iteritems():
+        for param_name, value in parameter_definition.items():
             if isinstance(value, dict) and "Default" in value:
                 default_values[param_name] = value["Default"]
 
diff --git a/samtranslator/validator/validator.py b/samtranslator/validator/validator.py
@@ -1,7 +1,10 @@
 import json
+
 import jsonschema
 from jsonschema.exceptions import ValidationError
-import sam_schema
+
+from . import sam_schema
+
 
 class SamTemplateValidator(object):
 
diff --git a/tests/policy_template_processor/test_processor.py b/tests/policy_template_processor/test_processor.py
diff --git a/tests/translator/output/error_globals_unsupported_type.json b/tests/translator/output/error_globals_unsupported_type.json
diff --git a/tests/translator/output/error_multiple_resource_errors.json b/tests/translator/output/error_multiple_resource_errors.json
diff --git a/tests/translator/test_function_resources.py b/tests/translator/test_function_resources.py
diff --git a/tests/translator/test_logical_id_generator.py b/tests/translator/test_logical_id_generator.py
diff --git a/tests/translator/test_translator.py b/tests/translator/test_translator.py
diff --git a/tox.ini b/tox.ini
