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

Add unicode_literals import #40637

Merged
merged 1 commit into from
Apr 12, 2017
Merged

Conversation

twangboy
Copy link
Contributor

What does this PR do?

Fixes Unicode issues in the log.debug messages in the win_wua module

What issues does this PR fix or reference?

#30980

Previous Behavior

Failing with a stack trace with Unicode characters in the title.

Tests written?

No

@cachedout
Copy link
Contributor

@twangboy Did you see the most recent comment here?

You may want to bring @cdunklau in on this.

@twangboy
Copy link
Contributor Author

@cachedout I did, but it seemed to reference older versions of Python. Since Windows stays pretty current, I didn't think it was an issue. Shall I do the u'string' approach instead?

@cdunklau
Copy link

I'd love to help, but I know very little about the situation in Windows, or really in general when it concerns large projects... and since I know very, very little about the Salt codebase, I don't think I'll be of much help. I posted that comment mainly as a warning, I didn't intend it to be a hard "hey don't do this".

The issues caused by unicode_literals in polyglot code seem to revolve around certain Python APIs that expect the "native" str type in both versions, especially those dealing with paths.

Explicitly using bytes/unicode literals without unicode_literals lets one choose one or the other where appropriate, while still keeping the ability to use "native" str where necessary.

@damon-atkins
Copy link
Contributor

This has been add to other windows execution modules from __future__ import unicode_literals
You can also use PY3 syntax to set the type.

There is also this recent change #40307 System encoding detection compatibility for windows

@cachedout cachedout merged commit 0638418 into saltstack:2016.11 Apr 12, 2017
@twangboy twangboy deleted the fix_unicode_issues branch June 14, 2017 21:21
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.

4 participants