Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix KMS lookup for Python2 #659

Merged
merged 1 commit into from
Sep 7, 2018
Merged

Fix KMS lookup for Python2 #659

merged 1 commit into from
Sep 7, 2018

Conversation

russellballestrini
Copy link
Member

Explicitly get an UTF-8 bytestring but don't try to decode anything afterwards.

Resolves this error:

  'ascii' codec can't decode byte 0xcb in position 5: ordinal not in range(128))
modified:   stacker/lookups/handlers/kms.py
modified:   stacker/tests/lookups/handlers/test_kms.py

Instead of implicitly getting an ASCII bytestring,
Explicitly get an UTF-8 bytestring.

Don't try to decode anything afterwards.

Resolves this error:

```
  'ascii' codec can't decode byte 0xcb in position 5: ordinal not in range(128))
```

	modified:   stacker/lookups/handlers/kms.py
	modified:   stacker/tests/lookups/handlers/test_kms.py
@russellballestrini
Copy link
Member Author

Verified to fix the issue found at @ Remind

Copy link
Contributor

@ejholmes ejholmes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks fine to me, but probably hold off on merging until @phobologic has had a chance to review. I don't really understand why explicitly encoding to utf-8 is necessary in 1.4, and not in 1.3, so I'm not sure if this would be better done in another location (like, should the entire parsed template be explicitly encoded to utf-8?)

@russellballestrini
Copy link
Member Author

I suppose it isn't necessary to be explicit but that's how the this bug crept in.

The trailing .decode was the actual issue.

@russellballestrini russellballestrini merged commit 11f9f5a into cloudtools:master Sep 7, 2018
@russellballestrini russellballestrini deleted the rb-fix-kms-unicode branch September 7, 2018 00:32
phrohdoh pushed a commit to phrohdoh/stacker that referenced this pull request Dec 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants