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 salt-cloud UnicodeEncodeError when writing to stdout #50236

Merged
merged 1 commit into from Nov 5, 2018

Conversation

Projects
None yet
4 participants
@rallytime
Copy link
Contributor

commented Oct 25, 2018

This is a partial back-port of #50146.

The full back-port was reverted due to the original change causing all of the salt-ssh tests to fail.

This change fixes the salt-cloud error, but allows salt-ssh to work correctly.

Refs #49523, #50146, #50174, #50231, and #50235

Fix salt-cloud UnicodeEncodeError when writing to stdout
This is a partial back-port of #50146.

The full back-port was reverted due to the original change causing all
of the salt-ssh tests to fail.

This change fixes the salt-cloud error, but allows salt-ssh to work
correctly.

Refs #49523, #50146, #50174, #50231, and #50235
@damon-atkins

This comment has been minimized.

Copy link
Member

commented Oct 26, 2018

From cmdmod the definition of use_vt (bool) -- Use VT utils (saltstack) to stream the command output more interactively to the console and the logs. This is experimental.

The following from cmdmod is the assumption the stdout/stderr contain text, not a data stream.

        try:
            out = salt.utils.stringutils.to_unicode(
                proc.stdout,
                encoding=output_encoding)
        except TypeError:
            # stdout is None
            out = ''
        except UnicodeDecodeError:
            out = salt.utils.stringutils.to_unicode(
                proc.stdout,
                encoding=output_encoding,
                errors='replace')
            if output_loglevel != 'quiet':
                log.error(
                    'Failed to decode stdout from command %s, non-decodable '
                    'characters have been replaced', cmd
                )

        try:
            err = salt.utils.stringutils.to_unicode(
                proc.stderr,
                encoding=output_encoding)
        except TypeError:
            # stderr is None
            err = ''
        except UnicodeDecodeError:
            err = salt.utils.stringutils.to_unicode(
                proc.stderr,
                encoding=output_encoding,
                errors='replace')
            if output_loglevel != 'quiet':
                log.error(
                    'Failed to decode stderr from command %s, non-decodable '
                    'characters have been replaced', cmd
                )

If you do not expect, text then output_encoding would need binary option to support binary and convert stdout to bytes.

Curiosity:
Q.1 Why is the solution convert text to bytes as encode() returns a bytes object
Q.2 Why is the solution not the same as cmdmod? What makes vt different?

See #45834 which added output_encoding option on my bequest.

@rallytime rallytime requested a review from gtmanfred Oct 29, 2018

@gtmanfred gtmanfred requested a review from terminalmage Oct 29, 2018

@cachedout cachedout merged commit 46c7dc2 into saltstack:2018.3 Nov 5, 2018

7 of 10 checks passed

jenkins/pr/lint The lint job has failed
Details
jenkins/pr/py2-windows-2016 The py2-windows-2016 job has failed
Details
jenkins/pr/py3-windows-2016 The py3-windows-2016 job has failed
Details
WIP Ready for review
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
jenkins/pr/docs The docs job has passed
Details
jenkins/pr/py2-centos-7 The py2-centos-7 job has passed
Details
jenkins/pr/py2-ubuntu-1604 The py2-ubuntu-1604 job has passed
Details
jenkins/pr/py3-centos-7 The py3-centos-7 job has passed
Details
jenkins/pr/py3-ubuntu-1604 The py3-ubuntu-1604 job has passed
Details

@rallytime rallytime deleted the rallytime:fix-cloud-vt branch Nov 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.