# \[ansible\] Proposing fix for Issue #4324; adding support for su (#5325)

**URL:** <https://forum.ansible.com/t/ansible-proposing-fix-for-issue-4324-adding-support-for-su-5325/14174>\
**Category:** Ansible Developer\
**Created:** [December 18, 2013, 3:37am UTC](https://forum.ansible.com/t/ansible-proposing-fix-for-issue-4324-adding-support-for-su-5325/14174 "2013-12-18T03:37:10Z")\
**Posts on this page:** 3\
**Page:** 1

<div class="post-metadata">

**Author:** ![Paul\_Durivage](https://avatars.discourse-cdn.com/v4/letter/p/94ad74/32.png) [@Paul\_Durivage](https://forum.ansible.com/u/Paul_Durivage)\
**Post date:** [December 18, 2013, 3:37am UTC](https://forum.ansible.com/t/ansible-proposing-fix-for-issue-4324-adding-support-for-su-5325/14174/1 "2013-12-18T03:37:10Z")

</div>

Hey Michael, Jerome, and the rest of the devel list.

I’m not very vocal on this list, and it seems everyone seems to do their best chatting on the mailing list, so I’m bringing my talk here. This about my pull request: [https://github.com/ansible/ansible/pull/5325](https://github.com/ansible/ansible/pull/5325)

Jerome, thanks for your input. I’ll review and adjust fire as necessary.

Michael, yes, you’re right, and I probably should have thought about this – that adding new things should attempt to reformat as little existing code as possible. The existing codebase is complex and has a lot of stuff going on. Since I’m a total newb/rookie, my strategy was to trace the existing sudo logic flow and add in support for su in a similar fashion, since they’re accomplishing mostly the same goals. This worked pretty well on the existing ssh.py, but ssh\_alt’s pipelining changed things, and while it’s not as pretty as it should be, it seems to be functional. This is the largest and most complex project I’ve contributed to, and I’m here to learn and contribute. Feedback is very welcome. Jerome brought up some good points in the PR that I hadn’t thought of, so I’ll probably drop some fixes in and try to be finished up by Friday this week.

Based on your comments, we’re going to beta this functionality as it’s implemented (a refactor). Jerome suggested breaking the functionality out into some, though I should admit I don’t have a vision for how this could or even should be accomplished.

So, my PR does include ssh and ssh\_alt. I excluded hostvars and the other conn plugins. What did I miss, if anything? What’s the path forward – what else can I do to close the loop and get things rolling?

---

<div class="post-metadata">

**Author:** ![Michael\_DeHaan2](https://avatars.discourse-cdn.com/v4/letter/m/da6949/32.png) [@Michael\_DeHaan2](https://forum.ansible.com/u/Michael_DeHaan2)\
**Post date:** [December 18, 2013, 4:09am UTC](https://forum.ansible.com/t/ansible-proposing-fix-for-issue-4324-adding-support-for-su-5325/14174/2 "2013-12-18T04:09:25Z")

</div>

It sounds like we’re in good shape as it is and we just need to test it on our end – and other testing absolutely welcome to.

My point re refactoring was not to do it with this addition – i.e. in this case we add the feature being comfortable with what it changes, and we can retool it later, I’m ok with the reformatting this introduces right now.

Offhand, possibly we might need some checks that su and sudo are mutually exclusive, which might be a possible user stumbling block.

---

<div class="post-metadata">

**Author:** ![Paul\_Durivage](https://avatars.discourse-cdn.com/v4/letter/p/94ad74/32.png) [@Paul\_Durivage](https://forum.ansible.com/u/Paul_Durivage)\
**Post date:** [December 18, 2013, 11:42pm UTC](https://forum.ansible.com/t/ansible-proposing-fix-for-issue-4324-adding-support-for-su-5325/14174/3 "2013-12-18T23:42:09Z")

</div>

Meant to send this to the list.
