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 python3 error in repoquery #4183

Merged
merged 1 commit into from
May 18, 2017
Merged

Conversation

jpeeler
Copy link

@jpeeler jpeeler commented May 12, 2017

Explicitly convert from bytes to string so that splitting the string is
successful. This change works with python 2 as well.

Closes #4182

Explicitly convert from bytes to string so that splitting the string is
successful. This change works with python 2 as well.

Closes openshift#4182
Copy link

@admiyo admiyo left a comment

Choose a reason for hiding this comment

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

Ran across this issue and the fix worked for me. If this is a python2 vs python3 error, consider using the Six module.

@abutcher abutcher requested a review from ashcrow May 16, 2017 15:59
Copy link
Member

@ashcrow ashcrow left a comment

Choose a reason for hiding this comment

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

This will work. Made a note about a possible future fix to avoid this happening in other lib_utils modules.

@@ -48,7 +48,7 @@ def process_versions(query_output):

version_dict = defaultdict(dict)

for version in query_output.split('\n'):
for version in query_output.decode().split('\n'):
Copy link
Member

Choose a reason for hiding this comment

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

This will be hit by other items using _run (local ref). It may be a better fix to decode() before _run returns.

Here is what was done in lib_openshift.

@@ -45,7 +45,7 @@ def test_querying_a_package(self, mock_cmd):

# Return values of our mocked function call. These get returned once per call.
mock_cmd.side_effect = [
(0, '4.2.46|21.el7_3|x86_64|rhel-7-server-rpms|4.2.46-21.el7_3', valid_stderr), # first call to the mock
(0, b'4.2.46|21.el7_3|x86_64|rhel-7-server-rpms|4.2.46-21.el7_3', valid_stderr), # first call to the mock
Copy link
Member

Choose a reason for hiding this comment

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

👍 as b'' in Python 2 will become a str while staying bytes in 3.

@ashcrow
Copy link
Member

ashcrow commented May 16, 2017

aos-ci-test

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.5_NOT_containerized, aos-ci-jenkins/OS_3.5_NOT_containerized_e2e_tests" for 7fb814d (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_containerized, aos-ci-jenkins/OS_3.6_containerized_e2e_tests" for 7fb814d (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_NOT_containerized, aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests" for 7fb814d (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.5_containerized, aos-ci-jenkins/OS_3.5_containerized_e2e_tests" for 7fb814d (logs)

@abutcher
Copy link
Member

[merge]

@abutcher
Copy link
Member

flake openshift/origin#10773

@abutcher
Copy link
Member

[merge]

@openshift-bot
Copy link

Evaluated for openshift ansible merge up to 7fb814d

@openshift-bot
Copy link

[test]ing while waiting on the merge queue

@openshift-bot
Copy link

Evaluated for openshift ansible test up to 7fb814d

@openshift-bot
Copy link

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_openshift_ansible/127/) (Base Commit: eedd16a)

@openshift-bot
Copy link

openshift-bot commented May 18, 2017

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_openshift_ansible/407/) (Base Commit: 2677014)

@openshift-bot openshift-bot merged commit befb34a into openshift:master May 18, 2017
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.

5 participants