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

util.py: make sure that the git commit ID is a string, not bytes #920

Merged
merged 1 commit into from
Feb 16, 2018

Conversation

owtaylor
Copy link
Contributor

subprocess.check_output() by default returns bytes for python3. Using
universal_newlines=True is a trick to get strings for both python2
and python3, since the encoding option is python3 only.

I hit this in testing atomic-reactor running with Python-3.6 - not sure why it hasn't previously been encountered since the code hasn't apparently changed for quite a while. The symptom that is seen is that the code in add_labels_in_df that escapes the environment values blows up if an environment value is bytes instead of string since b"Foo".translate() wants a different argument than "foo".translate().

owtaylor added a commit to owtaylor/osbs-box that referenced this pull request Feb 15, 2018
subprocess.check_output() by default returns bytes for python3. Using
universal_newlines=True is a trick to get strings for both python2
and python3, since the encoding option is python3 only.
Copy link
Member

@twaugh twaugh left a comment

Choose a reason for hiding this comment

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

Great catch, thanks!

@twaugh twaugh requested a review from rcerven February 16, 2018 10:02
@mlangsdorf mlangsdorf merged commit 58023cb into containerbuildsystem:master Feb 16, 2018
@twaugh
Copy link
Member

twaugh commented Feb 16, 2018

Updated draft release notes

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.

None yet

4 participants