From 88054895498a671de454628415455391d1f8cbef Mon Sep 17 00:00:00 2001
From: Jacob Fuss <32497805+jfuss@users.noreply.github.com>
Date: Thu, 24 May 2018 15:28:35 -0400
Subject: [PATCH 1/2] 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
---
 samtranslator/intrinsics/resolver.py                 |  8 ++++----
 samtranslator/model/__init__.py                      |  4 ++--
 samtranslator/model/eventsources/cloudwatchlogs.py   |  6 +++---
 samtranslator/model/exceptions.py                    |  5 ++++-
 samtranslator/model/function_policies.py             |  2 +-
 samtranslator/model/intrinsics.py                    |  2 +-
 .../preferences/deployment_preference_collection.py  | 10 +++++-----
 samtranslator/model/s3_utils/uri_parser.py           |  3 +--
 samtranslator/model/sam_resources.py                 | 11 ++++++-----
 samtranslator/model/update_policy.py                 |  2 +-
 samtranslator/plugins/api/implicit_api_plugin.py     |  4 ++--
 samtranslator/plugins/globals/globals.py             |  8 ++++----
 .../plugins/policies/policy_templates_plugin.py      |  6 +++---
 samtranslator/policy_template_processor/processor.py |  2 +-
 samtranslator/policy_template_processor/template.py  |  4 ++--
 samtranslator/region_configuration.py                |  3 ++-
 samtranslator/sdk/template.py                        |  2 +-
 samtranslator/swagger/swagger.py                     |  2 +-
 samtranslator/translator/logical_id_generator.py     |  4 +++-
 samtranslator/translator/translator.py               |  2 +-
 samtranslator/validator/validator.py                 |  5 ++++-
 tests/policy_template_processor/test_processor.py    |  4 ++--
 .../output/error_multiple_resource_errors.json       |  2 +-
 tests/translator/test_function_resources.py          | 12 ++++++------
 tests/translator/test_logical_id_generator.py        |  3 ++-
 tests/translator/test_translator.py                  |  2 +-
 26 files changed, 64 insertions(+), 54 deletions(-)

diff --git a/samtranslator/intrinsics/resolver.py b/samtranslator/intrinsics/resolver.py
index 99967a1cc..b50b28e2e 100644
--- 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
index f8a1169da..ecc354d6f 100644
--- 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
index b2dbb5bcb..4c4948f1e 100644
--- 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
index d22f34cbf..9db50a38c 100644
--- 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
index faeaec12f..d62c2dceb 100644
--- 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
index aae232aed..fd5e8e0e7 100644
--- 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
index 04fcb90a9..0f590793a 100644
--- 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,7 +53,7 @@ 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):
         """
@@ -61,14 +61,14 @@ def can_skip_service_role(self):
         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
index dc9385e3b..5452444fc 100644
--- 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
index cbe2bf252..c9f4c2c3d 100644
--- 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
index 4dfcc118a..30414e0f6 100644
--- 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
index e370b335a..807bc867b 100644
--- 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
index b50e697be..eed48d3ff 100644
--- a/samtranslator/plugins/globals/globals.py
+++ b/samtranslator/plugins/globals/globals.py
@@ -52,14 +52,14 @@ 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()]
+
         self.template_globals = {}
 
         if self._KEYWORD in template:
@@ -109,7 +109,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 +122,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
index 0560631e6..65c5653a9 100644
--- 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
index 13fefb6e7..54168f0e6 100644
--- 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
index e33a894ad..973c3b089 100644
--- 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
index 221186841..0d474b2e0 100644
--- 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
index 4cb615d99..c8136ee63 100644
--- 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
index eb49cdbf5..bce74c55a 100644
--- 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
index 4032ded9c..f04f32908 100644
--- a/samtranslator/translator/logical_id_generator.py
+++ b/samtranslator/translator/logical_id_generator.py
@@ -2,6 +2,7 @@
 import json
 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
@@ -55,7 +56,8 @@ def get_hash(self, length=HASH_LENGTH):
 
         data_hash = ""
         if self.data_str:
-            data_hash = hashlib.sha1(bytes(self.data_str)).hexdigest()
+            utf_encoded_data_str = str(self.data_str).encode("utf8")
+            data_hash = hashlib.sha1(bytes(utf_encoded_data_str)).hexdigest()
 
         return data_hash[:length]
 
diff --git a/samtranslator/translator/translator.py b/samtranslator/translator/translator.py
index 6a62a1804..57f6c187f 100644
--- 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
index 8ef8f8832..69f021af8 100644
--- 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
index 344c0e491..207f1208f 100644
--- a/tests/policy_template_processor/test_processor.py
+++ b/tests/policy_template_processor/test_processor.py
@@ -60,7 +60,7 @@ def test_init_must_convert_template_value_dict_to_object(self, template_from_dic
         processor = PolicyTemplatesProcessor(policy_templates_dict)
 
         self.assertEquals(2, len(processor.policy_templates))
-        self.assertEquals(set(["key1", "key2"]), set(processor.policy_templates.viewkeys()))
+        self.assertEquals(set(["key1", "key2"]), set(processor.policy_templates.keys()))
 
         # Template.from_dict must be called only once for each template entry
         self.assertEquals(2, template_from_dict_mock.call_count)
@@ -241,7 +241,7 @@ def test_is_valid_templates_dict_must_raise_for_invalid_input(self, read_schema_
             PolicyTemplatesProcessor._is_valid_templates_dict(policy_templates_dict)
 
         ex = cm.exception
-        self.assertEquals(ex.message, exception_msg)
+        self.assertEquals(str(ex), exception_msg)
 
     @patch.object(jsonschema, "validate")
     @patch.object(PolicyTemplatesProcessor, "_read_schema")
diff --git a/tests/translator/output/error_multiple_resource_errors.json b/tests/translator/output/error_multiple_resource_errors.json
index 5178814da..bdc58656f 100644
--- a/tests/translator/output/error_multiple_resource_errors.json
+++ b/tests/translator/output/error_multiple_resource_errors.json
@@ -16,5 +16,5 @@
       "errorMessage": "Resource with id [BadDefinitionUriTypeApi] is invalid. 'DefinitionUri' requires Bucket and Key properties to be specified"
     }
   ],
-  "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 5. Resource with id [BadCodeUriTypeFunction] is invalid. 'CodeUri' requires Bucket and Key properties to be specified Resource with id [ExternalS3Function] is invalid. Event with id [S3Event] is invalid. S3 events must reference an S3 bucket in the same template. Resource with id [BadCodeUriFunction] is invalid. 'CodeUri' is not a valid S3 Uri of the form \"s3://bucket/key\" with optional versionId query parameter. Resource with id [BadDefinitionUriApi] is invalid. 'DefinitionUri' is not a valid S3 Uri of the form \"s3://bucket/key\" with optional versionId query parameter. Resource with id [BadDefinitionUriTypeApi] is invalid. 'DefinitionUri' requires Bucket and Key properties to be specified"
+  "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 5. Resource with id [BadCodeUriFunction] is invalid. 'CodeUri' is not a valid S3 Uri of the form \"s3://bucket/key\" with optional versionId query parameter. Resource with id [BadCodeUriTypeFunction] is invalid. 'CodeUri' requires Bucket and Key properties to be specified Resource with id [BadDefinitionUriApi] is invalid. 'DefinitionUri' is not a valid S3 Uri of the form \"s3://bucket/key\" with optional versionId query parameter. Resource with id [BadDefinitionUriTypeApi] is invalid. 'DefinitionUri' requires Bucket and Key properties to be specified Resource with id [ExternalS3Function] is invalid. Event with id [S3Event] is invalid. S3 events must reference an S3 bucket in the same template."
 }
\ No newline at end of file
diff --git a/tests/translator/test_function_resources.py b/tests/translator/test_function_resources.py
index a547a279c..f3b4f4bce 100644
--- a/tests/translator/test_function_resources.py
+++ b/tests/translator/test_function_resources.py
@@ -60,7 +60,7 @@ def test_sam_function_with_alias(self, get_resolved_alias_name_mock):
         self.assertEquals(len(aliases), 1)
         self.assertEquals(len(versions), 1)
 
-        alias = aliases[0].values()[0]["Properties"]
+        alias = list(aliases[0].values())[0]["Properties"]
         self.assertEquals(alias["Name"], alias_name)
         # We don't need to do any deeper validation here because there is a separate SAM template -> CFN template conversion test
         # that will care of validating all properties & connections
@@ -109,8 +109,8 @@ def test_sam_function_with_deployment_preference(self, get_resolved_alias_name_m
 
         aliases = [r.to_dict() for r in resources if r.resource_type == LambdaAlias.resource_type]
 
-        self.assertTrue("UpdatePolicy" in aliases[0].values()[0])
-        self.assertEqual(aliases[0].values()[0]["UpdatePolicy"], self.update_policy().to_dict())
+        self.assertTrue("UpdatePolicy" in list(aliases[0].values())[0])
+        self.assertEqual(list(aliases[0].values())[0]["UpdatePolicy"], self.update_policy().to_dict())
 
     @patch.object(SamFunction, "_get_resolved_alias_name")
     def test_sam_function_with_deployment_preference_missing_collection_raises_error(self, get_resolved_alias_name_mock):
@@ -177,7 +177,7 @@ def test_sam_function_with_disabled_deployment_preference_does_not_add_update_po
         self.intrinsics_resolver_mock.resolve_parameter_refs.assert_called_with(enabled)
         aliases = [r.to_dict() for r in resources if r.resource_type == LambdaAlias.resource_type]
 
-        self.assertTrue("UpdatePolicy" not in aliases[0].values()[0])
+        self.assertTrue("UpdatePolicy" not in list(aliases[0].values())[0])
 
     def test_sam_function_cannot_be_with_deployment_preference_without_alias(self):
         with self.assertRaises(InvalidResourceException):
@@ -265,8 +265,8 @@ def test_sam_function_with_deployment_preference_instrinsic_ref_enabled_boolean_
 
         aliases = [r.to_dict() for r in resources if r.resource_type == LambdaAlias.resource_type]
 
-        self.assertTrue("UpdatePolicy" in aliases[0].values()[0])
-        self.assertEqual(aliases[0].values()[0]["UpdatePolicy"], self.update_policy().to_dict())
+        self.assertTrue("UpdatePolicy" in list(aliases[0].values())[0])
+        self.assertEqual(list(aliases[0].values())[0]["UpdatePolicy"], self.update_policy().to_dict())
 
     @patch.object(SamFunction, "_get_resolved_alias_name")
     def test_sam_function_with_deployment_preference_instrinsic_ref_enabled_dict_parameter(self, get_resolved_alias_name_mock):
diff --git a/tests/translator/test_logical_id_generator.py b/tests/translator/test_logical_id_generator.py
index a94d8b8e4..e6758bc4b 100644
--- a/tests/translator/test_logical_id_generator.py
+++ b/tests/translator/test_logical_id_generator.py
@@ -100,7 +100,8 @@ def testget_hash(self, stringify_mock):
         # We are essentially duplicating the implementation here. This is done on purpose to prevent
         # accidental change of the algorithm. Any changes to the hash generation must be backwards compatible.
         # This test will help catch such issues before hand.
-        expected = hashlib.sha1(bytes(stringified_data)).hexdigest()[:10]
+        utf_data = str(stringified_data).encode("utf8")
+        expected = hashlib.sha1(bytes(utf_data)).hexdigest()[:10]
         self.assertEqual(expected, generator.get_hash())
 
         stringify_mock.assert_called_once_with(data)
diff --git a/tests/translator/test_translator.py b/tests/translator/test_translator.py
index 40810b5b6..a674f23af 100644
--- a/tests/translator/test_translator.py
+++ b/tests/translator/test_translator.py
@@ -215,7 +215,7 @@ def test_transform_unhandled_failure_empty_managed_policy_map():
     with pytest.raises(Exception) as e:
         transform(document, parameter_values, mock_policy_loader)
 
-    error_message = e.value.message
+    error_message = str(e.value)
 
     assert error_message == 'Managed policy map is empty, but should not be.'
 

From e54a1bead908397b7150b2f34d4fbbea9af3a234 Mon Sep 17 00:00:00 2001
From: Sanath Kumar Ramesh <sanathkr@amazon.com>
Date: Tue, 12 Jun 2018 16:05:26 -0700
Subject: [PATCH 2/2] 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
---
 .travis.yml                                   |  12 +-
 samtranslator/plugins/globals/globals.py      |   2 +
 .../translator/logical_id_generator.py        |  17 ++-
 .../error_globals_unsupported_type.json       |   4 +-
 tests/translator/test_translator.py           | 135 +++++++++++++++++-
 tox.ini                                       |  44 +++---
 6 files changed, 181 insertions(+), 33 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index e2090bb66..41bc941e7 100644
--- 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/plugins/globals/globals.py b/samtranslator/plugins/globals/globals.py
index eed48d3ff..5c94009f5 100644
--- a/samtranslator/plugins/globals/globals.py
+++ b/samtranslator/plugins/globals/globals.py
@@ -59,6 +59,8 @@ def __init__(self, template):
         :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 = {}
 
diff --git a/samtranslator/translator/logical_id_generator.py b/samtranslator/translator/logical_id_generator.py
index f04f32908..30022c95e 100644
--- a/samtranslator/translator/logical_id_generator.py
+++ b/samtranslator/translator/logical_id_generator.py
@@ -1,5 +1,6 @@
 import hashlib
 import json
+import sys
 from six import string_types
 
 
@@ -55,9 +56,19 @@ def get_hash(self, length=HASH_LENGTH):
         """
 
         data_hash = ""
-        if self.data_str:
-            utf_encoded_data_str = str(self.data_str).encode("utf8")
-            data_hash = hashlib.sha1(bytes(utf_encoded_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/tests/translator/output/error_globals_unsupported_type.json b/tests/translator/output/error_globals_unsupported_type.json
index d49924ddc..6ed75010d 100644
--- a/tests/translator/output/error_globals_unsupported_type.json
+++ b/tests/translator/output/error_globals_unsupported_type.json
@@ -1,8 +1,8 @@
 {
   "errors": [
     {
-      "errorMessage": "'Globals' section is invalid. 'NewType' is not supported. Must be one of the following values - ['Function', 'SimpleTable', Api']"
+      "errorMessage": "'Globals' section is invalid. 'NewType' is not supported. Must be one of the following values - ['Api', 'Function', 'SimpleTable']"
     }
   ],
-  "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. 'Globals' section is invalid. 'NewType' is not supported. Must be one of the following values - ['Function', 'SimpleTable', 'Api']"
+  "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. 'Globals' section is invalid. 'NewType' is not supported. Must be one of the following values - ['Api', 'Function', 'SimpleTable']"
 }
\ No newline at end of file
diff --git a/tests/translator/test_translator.py b/tests/translator/test_translator.py
index a674f23af..f6e6e393c 100644
--- a/tests/translator/test_translator.py
+++ b/tests/translator/test_translator.py
@@ -1,7 +1,9 @@
 import json
 import itertools
 import os.path
-from functools import reduce
+import hashlib
+import sys
+from functools import reduce, cmp_to_key
 
 from samtranslator.translator.translator import Translator, prepare_plugins, make_policy_template_for_function_plugin
 from samtranslator.parser.parser import Parser
@@ -25,14 +27,63 @@
 output_folder = 'tests/translator/output'
 
 
-def deep_sorted(value):
+def deep_sort_lists(value):
+    """
+    Custom sorting implemented as a wrapper on top of Python's built-in ``sorted`` method. This is necessary because
+    the previous behavior assumed lists were unordered. As part of migration to Py3, we are trying to
+    retain the same behavior. But in Py3, lists with complex data types like dict cannot be sorted. Hence
+    we provide a custom sort function that tries best sort the lists in a stable order. The actual order
+    does not matter as long as it is stable between runs.
+
+    This implementation assumes that the input was parsed from a JSON data. So it can have one of the
+    following types: a primitive type, list or other dictionaries.
+    We traverse the dictionary like how we would traverse a tree. If a value is a list, we recursively sort the members
+    of the list, and then sort the list itself.
+
+    This assumption that lists are unordered is a problem at the first place. As part of dropping support for Python2,
+    we should remove this assumption. We have to update SAM Translator to output lists in a predictable ordering so we
+    can assume lists are ordered and compare them.
+    """
     if isinstance(value, dict):
-        return {k: deep_sorted(v) for k, v in value.items()}
+        return {k: deep_sort_lists(v) for k, v in value.items()}
     if isinstance(value, list):
-        return sorted(deep_sorted(x) for x in value)
+        if sys.version_info.major < 3:
+            # Py2 can sort lists with complex types like dictionaries
+            return sorted((deep_sort_lists(x) for x in value))
+        else:
+            # Py3 cannot sort lists with complex types. Hence a custom comparator function
+            return sorted((deep_sort_lists(x) for x in value), key=cmp_to_key(custom_list_data_comparator))
     else:
         return value
 
+
+def custom_list_data_comparator(obj1, obj2):
+    """
+    Comparator function used to sort lists with complex data types in them. This is meant to be used only within the
+    context of sorting lists for use with unit tests.
+
+    Given any two objects, this function will return the "difference" between the two objects. This difference obviously
+    does not make sense for complex data types like dictionaries & list. This function implements a custom logic that
+    is partially borrowed from Python2's implementation of such a comparison:
+
+    * Both objects are dict: Convert them JSON strings and compare
+    * Both objects are comparable data types (ie. ones that have > and < operators): Compare them directly
+    * Objects are non-comparable (ie. one is a dict other is a list): Compare the names of the data types.
+      ie. dict < list because of alphabetical order. This is Python2's behavior.
+
+    """
+
+    if isinstance(obj1, dict) and isinstance(obj2, dict):
+        obj1 = json.dumps(obj1, sort_keys=True)
+        obj2 = json.dumps(obj2, sort_keys=True)
+
+    try:
+        return (obj1 > obj2) - (obj1 < obj2)
+    # In Py3 a TypeError will be raised if obj1 and obj2 are different types or uncomparable
+    except TypeError:
+        s1, s2 = type(obj1).__name__, type(obj2).__name__
+        return (s1 > s2) - (s1 < s2)
+
 # implicit_api, explicit_api, explicit_api_ref, api_cache tests currently have deployment IDs hardcoded in output file.
 # These ids are generated using sha1 hash of the swagger body for implicit
 # api and s3 location for explicit api.
@@ -140,7 +191,81 @@ def test_transform_success(self, testcase, partition_with_region):
 
         print(json.dumps(output_fragment, indent=2))
 
-        assert deep_sorted(output_fragment) == deep_sorted(expected)
+        # Only update the deployment Logical Id hash in Py3.
+        if sys.version_info.major >= 3:
+            self._update_logical_id_hash(expected)
+            self._update_logical_id_hash(output_fragment)
+
+        assert deep_sort_lists(output_fragment) == deep_sort_lists(expected)
+
+    def _update_logical_id_hash(self, resources):
+        """
+        Brute force method for updating all APIGW Deployment LogicalIds and references to a consistent hash
+        """
+        output_resources = resources.get("Resources", {})
+        deployment_logical_id_dict = {}
+        rest_api_to_swagger_hash = {}
+        dict_of_things_to_delete = {}
+
+        # Find all RestApis in the template
+        for logical_id, resource_dict in output_resources.items():
+            if "AWS::ApiGateway::RestApi" == resource_dict.get("Type"):
+                resource_properties = resource_dict.get("Properties", {})
+                if "Body" in resource_properties:
+                    self._generate_new_deployment_hash(logical_id, resource_properties.get("Body"), rest_api_to_swagger_hash)
+
+                elif "BodyS3Location" in resource_dict.get("Properties"):
+                    self._generate_new_deployment_hash(logical_id,
+                                                       resource_properties.get("BodyS3Location"),
+                                                       rest_api_to_swagger_hash)
+
+        # Collect all APIGW Deployments LogicalIds and generate the new ones
+        for logical_id, resource_dict in output_resources.items():
+            if "AWS::ApiGateway::Deployment" == resource_dict.get("Type"):
+                resource_properties = resource_dict.get("Properties", {})
+
+                rest_id = resource_properties.get("RestApiId").get("Ref")
+
+                data_hash = rest_api_to_swagger_hash.get(rest_id)
+
+                description = resource_properties.get("Description")[:-len(data_hash)]
+
+                resource_properties["Description"] = description + data_hash
+
+                new_logical_id = logical_id[:-10] + data_hash[:10]
+
+                deployment_logical_id_dict[logical_id] = new_logical_id
+                dict_of_things_to_delete[logical_id] = (new_logical_id, resource_dict)
+
+        # Update References to APIGW Deployments
+        for logical_id, resource_dict in output_resources.items():
+            if "AWS::ApiGateway::Stage" == resource_dict.get("Type"):
+                resource_properties = resource_dict.get("Properties", {})
+
+                rest_id = resource_properties.get("RestApiId", {}).get("Ref", "")
+
+                data_hash = rest_api_to_swagger_hash.get(rest_id)
+
+                deployment_id = resource_properties.get("DeploymentId", {}).get("Ref")
+                new_logical_id = deployment_logical_id_dict.get(deployment_id, "")[:-10]
+                new_logical_id = new_logical_id + data_hash[:10]
+
+                resource_properties.get("DeploymentId", {})["Ref"] = new_logical_id
+
+        # To avoid mutating the template while iterating, delete only after find everything to update
+        for logical_id_to_remove, tuple_to_add in dict_of_things_to_delete.items():
+            output_resources[tuple_to_add[0]] = tuple_to_add[1]
+            del output_resources[logical_id_to_remove]
+
+        # Update any Output References in the template
+        for output_key, output_value in resources.get("Outputs", {}).items():
+            if output_value.get("Ref") in deployment_logical_id_dict:
+                output_value["Ref"] = deployment_logical_id_dict[output_value.get("Ref")]
+
+    def _generate_new_deployment_hash(self, logical_id, dict_to_hash, rest_api_to_swagger_hash):
+        data_bytes = json.dumps(dict_to_hash, separators=(',', ':'), sort_keys=True).encode("utf8")
+        data_hash = hashlib.sha1(data_bytes).hexdigest()
+        rest_api_to_swagger_hash[logical_id] = data_hash
 
 
 @pytest.mark.parametrize('testcase', [
diff --git a/tox.ini b/tox.ini
index 20808b9eb..30760f497 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,26 +1,26 @@
+# tox (https://tox.readthedocs.io/) is a tool for running tests
+# in multiple virtualenvs. This configuration file will run the
+# test suite on all supported python versions. To use it, "pip install tox"
+# and then run "tox" from this directory.
+
 [tox]
-envlist=py27, flake8
+envlist = py27, py36
 
-[flake8]
-ignore = E126
+[testenv:py27]
+# Set this environment variable **only** for Python2.7. In Py >= 3.3, the hash seed property was set to a random
+# value on every execution. This is the seed for random number generator used to compute hash of objects. This hash
+# is widely used in ``dict`` to compute insertion position. In Py2.7, this seed is uninitialized so when iterating
+# over a dictionary, you tend to get the same order always. Unfortunately, this consistency is because the seed remains
+# same and not a ordering guarantee provided by the dictionary.
+#
+# SAM Translator has a dependency on this pseudo-ordering to generate a stable LogicalID for API Gateway Deployment
+# resource. Tox tries to simulate Py3 behavior in Py2.7 by setting PYTHONHASHSEED to random values on every run.
+# This results in unit test failures. This happens only within Tox. To fix this, we are unsetting the seed value
+# specifically for Py27 in Tox.
+passenv = AWS*
+setenv = PYTHONHASHSEED = 0
 
 [testenv]
-# pytest parameters are included in the pytest.ini
-commands=pytest
-deps=
-    coverage
-    pylint
-    pytest
-    py
-    pytest-cov
-    mock
-    nose
-    parameterized
-    requests
-
-[testenv:flake8]
-basepython = python2.7
-deps =
-    flake8
-commands =
-    flake8 samtranslator tests --max-line-length=120
\ No newline at end of file
+passenv = AWS*
+whitelist_externals = make
+commands = make pr