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

Handle bytes and strings from RPM (#1764642) #23

Merged
merged 1 commit into from Oct 23, 2019

Conversation

poncovka
Copy link
Contributor

@poncovka poncovka commented Oct 23, 2019

RPM can return bytes or strings, so let's use the function decode_bytes that
can handle both and always returns a string.

Based on: rhinstaller/anaconda@1c45e7b

Related: rhbz#1693766
Resolves: rhbz#1764642

RPM can return bytes or strings, so let's use the function decode_bytes that
can handle both and always returns a string.

Related: rhbz#1693766
Resolves: rhbz#1764642
@AdamWill
Copy link
Contributor

I beat you ;)

#22

both of these would work, I think.

@AdamWill
Copy link
Contributor

mind you, I think this one changes the behaviour in Python 2. It would leave the data as a str, whereas the current code decodes it to a unicode.

@AdamWill
Copy link
Contributor

also this defines the function after using it, which looks wrong.

@M4rtinK
Copy link
Contributor

M4rtinK commented Oct 23, 2019

mind you, I think this one changes the behaviour in Python 2. It would leave the data as a str, whereas the current code decodes it to a unicode.

We have dropped Python 3 support a while ago in python-meh, so this should not be an issue. Also I think this is more consistent with how we do this in Anaconda.

BTW, I think we should eventually require RPM >= 4.15 & drop all these hacks.

@AdamWill
Copy link
Contributor

I just figured it's safer to handle all eventualities in case this gets backported to RHEL 7 or something like that.

@M4rtinK
Copy link
Contributor

M4rtinK commented Oct 23, 2019

I just figured it's safer to handle all eventualities in case this gets backported to RHEL 7 or something like that.

Thanks for the consideration. :) In practices we have branches per RHEL major release and we manually adjust patches if they get backported from Fedora to RHEL (and in some rare cases in the opposite direction).

We could certainly try to have a single codebase that is compatible with all these environments, but we rather decided to have a clean & modern master branch without all the baggage needed for supporting multiple RHEL releases would entail, even at the cost of having to adjust patches as they flow between the branches.

This actually significantly helps when its time to get a new RHEL major release out of the door. :)

Copy link
Contributor

@M4rtinK M4rtinK left a comment

Choose a reason for hiding this comment

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

Looks good to me & thanks! :)

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

3 participants