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

Use a pty for local connections when the command is sudoable #39024

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
5 participants
@jimi-c
Member

jimi-c commented Apr 19, 2018

Fixes #38696

@jimi-c

This comment has been minimized.

Member

jimi-c commented Apr 19, 2018

!bot_skip

@abadger

This comment has been minimized.

Member

abadger commented Apr 19, 2018

Is there a drawback to using a pty? If not, then we should use it always. That way there's less code paths where a bug can make something break. (If there is a drawback, then perhaps we want to add a comment to be more restrictive in the future (only use it for the become methods where it's required).

@jimi-c

This comment has been minimized.

Member

jimi-c commented Apr 19, 2018

I believe using a pty consumes file handles (could be wrong), but didn't want to impinge on the number of files further if we didn't have to.

@jimi-c jimi-c force-pushed the jimi-c:local_connection_pty branch to 792e76f Apr 19, 2018

@mkrizek mkrizek removed the needs_triage label Apr 20, 2018

@abadger

This comment has been minimized.

Member

abadger commented Apr 23, 2018

This looks good to me... Of course, there's lots of other connection plugins that might benefit from this as well (the chroot family for instance).

@mattclay

This comment has been minimized.

Member

mattclay commented Apr 26, 2018

CI failure due to timeouts: https://app.shippable.com/github/ansible/ansible/runs/62296/59/console

The connection_local integration test is hanging at:

TASK [remove remote file with unicode filename and content] ********************

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