feat: add support for Python 3 #428

jfuss · 2018-05-09T22:30:15Z

Issue #, if available:

Description of changes:
This is a work in progress. Py3 is not fully supported but sending this to get early feedback while I try to get the rest of the tests running in Py (3.6, specifically).

Details of the commit:
Most of this was drop in replacements for methods:

.iteritems() -> .items()
.itervalues() -> values()
.keys() returns a view into the dictionary in Py3 (list in Py2). So where needed, wrapped the .keys() in a list().
Updated imports to be relative for modules that were in the same folder.
Needed to do some trickery with strings due to differences in Py3 and Py2. This only impacts the LogicalIdGenerator but is the most concerning (imo) and biggest potential to cause deployments for Api Gateway when customers don't expect. Please look over this part with a fine tooth comb!

All tests pass in Py3 except for the majority of the 'functional tests' in the test_translator.py. This is caused by:

The deepsort is comparing elements of different types which is causing issues in Py3.
The strings we generate for the exceptions are non-deterministic between Py2 and Py3.

Ways to resolve what is left:

Move away from deepsort. This was written to use pytest and asserts. All new code is written using unittest where you can compare dict by self.assertEquals().
I need to spend more time and dig a little deeper here. At the moment I don't have a clean way of solving. Suggestions always welcome!

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Without moving this to the __init__, the globals.py file will not run in Py3 because it can't reference the constants.

In Py3, the function .iteritems() on a dict was removed and replaced with .items().

In Py3, .itervalues() and .iteritems() was replaced with .values() and .items(), respectfully.

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.

Updated .iteritems() to items()

brettstack · 2018-05-09T22:40:30Z

LGTM. Py2 tests are passing which is the main thing. I think we can merge this and address the remaining Py3 test issues in a separate PR.

brettstack · 2018-05-18T21:54:09Z

@sanathkr can we get an approval for this and merge it, knowing that there is still work to be done to get us all the way to Py3?

…in Py3

jfuss · 2018-05-21T19:52:04Z

samtranslator/translator/logical_id_generator.py

        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")


I think Py2 default encoding is latin1. This may need to be updated to that but unsure.

What is the type of self.data_str?

If it's bytes (data_str = b'test'), then str(data_str) gives:

"b'test'" on python 3

b'test' on python 2 (correct)

If it's unicode (data_str = u'snowman: \u2603'), then str(data_str):

gives u'snowman: \u2603' on python 3

raises a UnicodeEncodeError on python 2

I think what you actually want here is:

unicode = type(u'') # or from six import unicode s = self.data_str if isinstance(s, unicode): s = s.encode('utf-8')

Calling str here is likely to do bad things -

self.data_str is a string data - so in py2 it is instance of basestring and in py3 it is unicode. Your solution is correct. We should encode only if it is unicode and skip otherwise.

You could also just encode if sys.version_info.major >= 3.

@jfuss's remark about py2 default encoding is irrelvant here - data_str comes from json.dumps, which by default only outputs unicode codepoints that can be encoded to ascii

brettstack · 2018-05-21T20:21:36Z

samtranslator/model/exceptions.py

        self._logical_id = logical_id
        self._message = message

+    def __lt__(self, other):


What's this? Operator overloading in Python?

Yeah, that's a magic method in Python-speak: https://rszalski.github.io/magicmethods/

sanathkr

LGTM

eric-wieser · 2018-05-24T19:39:30Z

samtranslator/model/preferences/deployment_preference_collection.py

@@ -1,9 +1,9 @@
-from deployment_preference import DeploymentPreference
+from .deployment_preference import DeploymentPreference


You should add from __future__ import absolute_import, which will make this fail on python 2 as well

eric-wieser · 2018-05-24T19:40:39Z

tests/translator/test_function_resources.py

        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])


list(x.values())[0] is possibly faster as next(iter(x.values()))

eric-wieser · 2018-05-24T19:41:57Z

If you run python2 with the -3 argument, it will give warnings about any parts of the code that change behavior in python3

* 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: 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

Jacob Fuss added 9 commits May 8, 2018 20:08

refactor: Updated imports to by Py3 compliant

246c7ff

refactor: Move class variable creation to constructor in globals.py

3947a2c

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

b48a369

In Py3, the function .iteritems() on a dict was removed and replaced with .items().

refactor: Update deployment_preference_collection.py to be Py3 compliant

4ea89e9

In Py3, .itervalues() and .iteritems() was replaced with .values() and .items(), respectfully.

refactor: Update swagger.py to be Py3 compliant

13667cd

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

80013b5

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

6df0381

Updated .iteritems() to items()

refactor: More updating to be Py3 compliant

c96883d

refactor: Make hashing constisent between py2 and py3

793afca

jfuss mentioned this pull request May 17, 2018

Support Python3 aws/aws-sam-cli#387

Closed

refactor: Make exceptions sortable to allow error case tests to pass …

8456a22

…in Py3

jfuss commented May 21, 2018

View reviewed changes

brettstack approved these changes May 21, 2018

View reviewed changes

jfuss mentioned this pull request May 22, 2018

Correcting import errors when installed with aws-sam-cli 0.3.0 #443

Closed

brettstack self-assigned this May 22, 2018

brettstack mentioned this pull request May 22, 2018

[Small fix] SAM Translator install fails on Py3.2.3 or higher #412

Closed

brettstack changed the title ~~WIP: feat: Py3 support~~ feat: add support for Python 3 May 22, 2018

sanathkr changed the base branch from develop to py3-support May 24, 2018 19:25

sanathkr self-requested a review May 24, 2018 19:26

sanathkr approved these changes May 24, 2018

View reviewed changes

brettstack merged commit 5927fe9 into aws:py3-support May 24, 2018

eric-wieser reviewed May 24, 2018

View reviewed changes

sanathkr mentioned this pull request May 24, 2018

fix: Fixing unit tests to work in Python 3 #445

Merged

brettstack mentioned this pull request Jun 28, 2018

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

Merged

		@@ -1,9 +1,9 @@
		from deployment_preference import DeploymentPreference
		from .deployment_preference import DeploymentPreference

feat: add support for Python 3 #428

feat: add support for Python 3 #428

Uh oh!

Conversation

jfuss commented May 9, 2018

Uh oh!

brettstack commented May 9, 2018

Uh oh!

brettstack commented May 18, 2018

Uh oh!

jfuss May 21, 2018

Choose a reason for hiding this comment

Uh oh!

eric-wieser May 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sanathkr May 24, 2018

Choose a reason for hiding this comment

Uh oh!

eric-wieser May 25, 2018

Choose a reason for hiding this comment

Uh oh!

brettstack May 21, 2018

Choose a reason for hiding this comment

Uh oh!

adamn May 24, 2018

Choose a reason for hiding this comment

Uh oh!

sanathkr left a comment

Choose a reason for hiding this comment

Uh oh!

eric-wieser May 24, 2018

Choose a reason for hiding this comment

Uh oh!

eric-wieser May 24, 2018

Choose a reason for hiding this comment

Uh oh!

eric-wieser commented May 24, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

eric-wieser May 24, 2018 •

edited

Loading