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

Encode shell commands explicitly. #49322

merged 1 commit into from Aug 25, 2018


None yet
3 participants

dwoz commented Aug 24, 2018

What does this PR do?

Fixes this test failure:

  • Do not raise an exception if passing non ascii data to subprocess.Popen, encode the command first so that it's not implicitly encoded using the default codepage on Python 2. This should also fix the exception found in #48880
  • Account for the differences in the way py2 and py3 handles Popen output encoding.

If we want to support this stuff on python 2 we'll need to bypass the subprocess module and do something like we do in salt.utils.win_runas we may even be able to leverage the work already done there.

Tests written?


Commits signed with GPG?


Encode shell commands explicitly.
On Windows subprocess.Popen will try to implicity encode shell commands
using the default system encoding. This will raise an exception if
passed utf-8.

@dwoz dwoz requested a review from rallytime Aug 24, 2018

@dwoz dwoz merged commit 5766e98 into saltstack:2018.3 Aug 25, 2018

6 of 8 checks passed

continuous-integration/jenkins/pr-merge This commit cannot be built
jenkins/pr/py2-centos-7 The py2-centos-7 job has failed
WIP ready for review
jenkins/pr/docs The docs job has passed
jenkins/pr/lint The lint job has passed
jenkins/pr/py2-ubuntu-1604 The py2-ubuntu-1604 job has passed
jenkins/pr/py3-centos-7 The py3-centos-7 job has passed
jenkins/pr/py3-ubuntu-1604 The py3-ubuntu-1604 job has passed

rallytime added a commit that referenced this pull request Aug 28, 2018

@dwoz dwoz deleted the dwoz:state_module_test_fix branch Sep 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment