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

expect: utf-8' codec can't decode byte 0x80 enhancement #46556 #51505

Closed
wants to merge 4 commits into from

Conversation

madhu222
Copy link

@madhu222 madhu222 commented Jan 30, 2019

SUMMARY

Modified the expect.py script to handle the utf-8 codec error

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

lib/ansible/modules/commands/expect.py

ADDITIONAL INFORMATION

If the reload is called after the telnet, it will show the utf-8 codec error. If I do the codec_error as replace option, the error is not seen. The example is shown below.

- name: Telnet to the switch to get the prompt
  delegate_to: localhost
  become: no
  expect:
    command: telnet {{ ansible_console }} {{ console_port }} -e {{ telnet_esc }}
    timeout: "{{ timeout|default(30) }}"
    responses: "{{ exp | default({}) | combine(default_exp) | combine(extra_exp) }}"
    echo: no
    codec_errors: replace
  register: output

@ansibot
Copy link
Contributor

ansibot commented Jan 30, 2019

cc @sivel
click here for bot help

@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. support:community This issue/PR relates to code supported by the Ansible community. labels Jan 30, 2019
@ansibot

This comment has been minimized.

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Jan 30, 2019
Copy link
Member

@sivel sivel left a comment

Choose a reason for hiding this comment

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

In addition to the other comments, this will also require a changelog to changelog/fragments

lib/ansible/modules/commands/expect.py Show resolved Hide resolved
@@ -133,6 +133,7 @@ def main():
responses=dict(type='dict', required=True),
timeout=dict(type='int', default=30),
echo=dict(type='bool', default=False),
codec_errors=dict(type='str'),
Copy link
Member

Choose a reason for hiding this comment

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

This is missing documentation. In addition you should probably support choices and enumerate the possibilities provided by codecs.

Copy link
Author

Choose a reason for hiding this comment

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

@sivel Will add the documentation

@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. needs_triage Needs a first human triage before being processed. labels Jan 30, 2019
@ansibot
Copy link
Contributor

ansibot commented Jan 30, 2019

The test ansible-test sanity --test validate-modules [explain] failed with 2 errors:

lib/ansible/modules/commands/expect.py:0:0: E309 version_added for new option (codec_errors) should be 2.8. Currently 0.0
lib/ansible/modules/commands/expect.py:0:0: E324 Value for "default" from the argument_spec (None) for "codec_errors" does not match the documentation ('False')

click here for bot help

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Jan 30, 2019
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Feb 8, 2019
@gundalow gundalow changed the title utf-8' codec can't decode byte 0x80 enhancement in expect script #46556 expect: utf-8' codec can't decode byte 0x80 enhancement #46556 Feb 21, 2019
lib/ansible/modules/commands/expect.py Show resolved Hide resolved
lib/ansible/modules/commands/expect.py Outdated Show resolved Hide resolved
@gundalow gundalow added feature This issue/PR relates to a feature request. and removed bug This issue/PR relates to a bug. labels Feb 21, 2019
gundalow and others added 2 commits March 7, 2019 09:54
Co-Authored-By: madhu222 <informmadhu@gmail.com>
Co-Authored-By: madhu222 <informmadhu@gmail.com>
@ansibot ansibot added commands Commands category and removed ci_verified Changes made in this PR are causing tests to fail. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Mar 7, 2019
@ansibot
Copy link
Contributor

ansibot commented Mar 7, 2019

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/commands/expect.py:147:59: bad-whitespace Exactly one space required after comma             codec_errors=dict(type='str', choices=['strict','replace']),                                                            ^

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/modules/commands/expect.py:147:60: E231 missing whitespace after ','

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/commands/expect.py:0:0: E324 Argument 'codec_errors' in argument_spec defines default as (None) but documentation defines default as ('False')

click here for bot help

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Mar 7, 2019
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Mar 15, 2019
@ansibot ansibot added support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed support:community This issue/PR relates to code supported by the Ansible community. labels Mar 27, 2020
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label May 16, 2020
@Akasurde
Copy link
Member

@madhu222 Are you still working on this? Thanks.

@ansibot ansibot added pre_azp This PR was last tested before migration to Azure Pipelines. and removed ci_verified Changes made in this PR are causing tests to fail. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Dec 6, 2020
@bcoca
Copy link
Member

bcoca commented Aug 6, 2021

close via alternate fix in #73255

@mkrizek mkrizek closed this Aug 6, 2021
@ansible ansible locked and limited conversation to collaborators Sep 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.8 This issue/PR affects Ansible v2.8 commands Commands category feature This issue/PR relates to a feature request. has_issue module This issue/PR relates to a module. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. new_contributor This PR is the first contribution by a new community member. pre_azp This PR was last tested before migration to Azure Pipelines. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants