Move pexpect from a runtime dependency to test #2921
Conversation
We do not really need pexpect as a runtime dependency as it was using only inside login. Using just subprocess.run() seems to work fine, so we can lower our dependencies.
| cmd = "/usr/bin/env {}".format(login_cmd) | ||
| self._pt = pexpect.spawn(cmd, dimensions=dimensions) |
There was a problem hiding this comment.
@retr0h Can you please comment on why the pexpect implementation was added in the first place for the interactive login? I am asking because I would not want to introduce a serious regression just because I do not know the reasons behind original implementation.
I did some basic tests and using subprocess.run() appeared to be working fine for me.
There was a problem hiding this comment.
Possible reason from the docs: "The run() function was added in Python 3.5"
If we previously supported < 3.5, then this call wouldn't have been possible.
There was a problem hiding this comment.
Obviously pending input from @retr0h, but it looks good to me.
| cmd = "/usr/bin/env {}".format(login_cmd) | ||
| self._pt = pexpect.spawn(cmd, dimensions=dimensions) |
There was a problem hiding this comment.
Possible reason from the docs: "The run() function was added in Python 3.5"
If we previously supported < 3.5, then this call wouldn't have been possible.
We do not really need pexpect as a runtime dependency as it was using only inside login. Using just subprocess.run() seems to work fine, so we can lower our dependencies.