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.

Sign up for GitHub

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

Jump to bottom

Allow request_exec with async I/O and more #576

Merged

webknjaz merged 8 commits into ansible:devel from pbrezina:extras

Feb 6, 2024

Contributor

pbrezina commented Feb 2, 2024 •

edited by webknjaz

Loading

This PR add calls to libssh request_exec() and relevant functions in order
to allow non-blocking async I/O. I need this to replace parallel-ssh which
unfortunately became a dead project, incompatible with Python 3.12+.

Additionally, it adds call to libssh request_send_signal() which allows to
send signals to the remote process if SSH server supports it.

I also fixed few bugs that I stumbled upon.

SUMMARY

$sbj.

ISSUE TYPE

Bugfix Pull Request
Docs Pull Request
Feature Pull Request

ADDITIONAL INFORMATION

N/A

pbrezina force-pushed the extras branch 3 times, most recently from e166d74 to 92572c3 Compare

February 2, 2024 15:53

psf-chronographer bot added the bot:chronographer:provided label

pbrezina force-pushed the extras branch from acf2699 to 687a08f Compare

February 2, 2024 16:02

webknjaz reviewed

View reviewed changes

docs/changelog-fragments/576.bugfix.rst Outdated Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

docs/changelog-fragments/576.bugfix.rst Outdated Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

docs/changelog-fragments/576.bugfix.rst Outdated Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

tests/unit/channel_test.py Outdated Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

tests/unit/channel_test.py Outdated Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

tests/unit/channel_test.py Outdated Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

src/pylibsshext/channel.pyx Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

src/pylibsshext/channel.pyx Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

src/pylibsshext/channel.pyx Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

src/pylibsshext/channel.pyx Outdated Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

src/pylibsshext/channel.pyx Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

src/pylibsshext/channel.pyx Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

docs/changelog-fragments/576.feature.rst Outdated Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

docs/changelog-fragments/576.feature.rst Outdated Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

docs/changelog-fragments/576.doc.rst Outdated Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

tests/unit/channel_test.py Outdated Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

tests/unit/channel_test.py Outdated Show resolved Hide resolved

pbrezina force-pushed the extras branch 5 times, most recently from de1d4eb to 87fc2bd Compare

February 5, 2024 12:16

Contributor Author

pbrezina commented Feb 5, 2024

It is ready for next round of review. I addresses all comments, leaving few as "unresolved" in case you want to discuss it further.

webknjaz reviewed

View reviewed changes

src/pylibsshext/channel.pyx Outdated Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

docs/changelog-fragments/576.bugfix.rst Outdated Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

src/pylibsshext/channel.pyx Outdated Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

src/pylibsshext/channel.pyx Outdated Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

tests/unit/channel_test.py Show resolved Hide resolved

webknjaz reviewed

View reviewed changes

tests/conftest.py Show resolved Hide resolved

pbrezina force-pushed the extras branch 2 times, most recently from 6561702 to 822f443 Compare

February 6, 2024 10:11

pbrezina added 7 commits

February 6, 2024 11:16


          Add low level call to request_exec and send_eof

7b8ba12

To allow asynchronous handling of the execution.


          Add direct call to request_send_signal

dadb7dd

OpenSSH now supports receiving signals for a long time. This is
useful when you need to communicate with a process via signals.


          Handle EOF in recv_nonblocking

6d13cfe

If recv_nonblocking is called and the ssh command ended with EOF
and empty output, the following error is produced because nbytes ==
SSS_EOF == -127:

```
File "src/pylibsshext/channel.pyx", line 112, in pylibsshext.channel.Channel.read_nonblocking
SystemError: Negative size passed to PyBytes_FromStringAndSize
```

It is better to handle it gracefully.


          Add direct call to is_eof

0b1a5ea

When doing asynchronous I/O it is usefull to know when EOF is hit
after polling for data.


          Keep reference of Session in Channel

b78f404

It may happen that a garbage collector collects Session object befor
unclosed Channel. Then Channel's destructor tries to close the channel,
libssh session is accessed but it is already freed, thus causing
segfault.

Keeping reference to the Session object makes sure that Session
is collected after Channel.


          docs: fix exec_command example

0b9111f


          Add changelog fragments

4d1df86

pbrezina force-pushed the extras branch from 822f443 to 4d1df86 Compare

February 6, 2024 10:16

Contributor Author

pbrezina commented Feb 6, 2024

Thank you. It is ready for next round.

pbrezina mentioned this pull request

Find alternative to parallel-ssh next-actions/pytest-mh#26

Closed

webknjaz mentioned this pull request

[BUG] Flaky SEGFAULT while testing exec_command #57

Open

webknjaz approved these changes

View reviewed changes

Member

webknjaz left a comment

Thank you for the well-explained patch!

webknjaz reviewed

View reviewed changes

tests/unit/channel_test.py Show resolved Hide resolved


          tests: fix random timeout when connecting to ssh

376fc52

Previous implementation of wait_for_svc_ready_state was not robust
enough as it could return even if further ssh connections kept failing.

webknjaz merged commit 80f74e5 into ansible:devel

144 of 163 checks passed

pbrezina mentioned this pull request

ssh: switch from parallel-ssh to ansible-pylibssh next-actions/pytest-mh#44

Merged

webknjaz pushed a commit that referenced this pull request


          Require openssh-clients for build @ RPM spec

9128f3b

As of #576, `ssh` client started being used in tests to ensure that
sshd is ready to receive connectoins. That PR failed to include this
patch so it's being applied separately.

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

Labels

bot:chronographer:provided