:thumbsup:

I know this code needs adjustment, but it would help to be clear exactly what we're trying to address. What is the bug here?

I'm not sure that this is the right fix. I'm not sure if ignoring keys is enough. I think there is something more fundamental wrong here.

2 of the 3 places that use FixEncodingRecurse only do so in the py2 case. A number of the fixEncoding cases are also py2 only. I'm wondering if we shouldn't be leaving these strings alone in py3

Ah, reference got lost in PR. It is related to #1282 (reference in last commit).
I was also thinking, that py3 probably solved the problem of fixEncoding's existence. But still not sure about different combinations of hub and client py2/3 versions.

@mikem - I've removed most of the code and tried with bypassing fixEncoding for py3. Not sure about that, look especially at differences in test code. On the other hand it allowed me to remove some py2/3 switch in other parts of code. we can extend if for removeNonprintable if current code makes some sense.

The asymmetry in honoring remove_nonprintable for PY3 seems odd.

You've got an unrelated fix in there. It looks good. I don't know if it's worth pulling it out, but we should at least have an issue to track it.

The asymmetry in honoring remove_nonprintable for PY3 seems odd.

We definitely need to honor remove_nonprintable for fixEncodingRecurse. This option is used by getRPMHeaders and getChangelogEntries. In both cases, this was added to work around problem characters that could not be encoded in xmlrpc. Stripping those chars may not have been the best solution, but we need to preserve it until we have a better one.

This update adds support for remove_nonprintable to fixEncodingRecurse under python3.

https://github.com/mikem23/koji-playground/commits/pagure/pr/1293

The refactor may be slightly overkill here, but I've been meaning to do this ever since DataWalker was added. Less code, more readable.

Alternate solution:

https://github.com/mikem23/koji-playground/commits/pr1293b

A smaller change from where we are. Probably safer

Either solution is good to me, though I think the first option is better simply because it is much easier to understand.

I've filed #1318 to track the fixEncodingRecurse refactor

Commit 2250daba fixes this pull-request