Skip to content

Make remote runtime fully async with aiohttp + testing#214

Merged
klieret merged 6 commits intomainfrom
rebased-async-remote-runtime
Aug 6, 2025
Merged

Make remote runtime fully async with aiohttp + testing#214
klieret merged 6 commits intomainfrom
rebased-async-remote-runtime

Conversation

@klieret
Copy link
Member

@klieret klieret commented Jun 27, 2025

  • Make remote runtime fully async with aiohttp
  • [pre-commit.ci] auto fixes from pre-commit.com hooks
  • CI: Properly cleanup sessions

@klieret klieret changed the title rebased async remote runtime (rebased for testing) Jun 27, 2025
@klieret klieret changed the title (rebased for testing) (rebased #197 for testing) Jun 27, 2025
saltzm and others added 5 commits July 1, 2025 14:30
@saltzm saltzm force-pushed the rebased-async-remote-runtime branch from 7b0c949 to 818d7d6 Compare July 1, 2025 14:37
@saltzm
Copy link
Collaborator

saltzm commented Jul 1, 2025

@klieret I found the issue:

  1. The server closes the connection because the FileNotFoundException is seen as "unhandled". This was true in the old code as well.
  2. We try to reuse the same aiohttp session to close the runtime during test teardown, but that session is associated with a closed connection, and so the close request fails with Connection reset by peer

In the old version of the code, the requests library was being used, which always creates a new session per request, so it would just recreate a fresh connection for the close request, and avoid this issue.

So I just changed the new code to match the old behavior and always create a new session per request.

This fixed the non_existent_file tests, but locally at least there are 18 failures in tests/test_execution.py that also exist in the main branch:

FAILED tests/test_execution.py::test_execute_command_with_empty_string_in_session - AssertionError: assert '\r\n\x1bkec2...ReX\x1b\\\r\n' == '\n'
FAILED tests/test_execution.py::test_execute_command_with_newline_in_session - AssertionError: assert '\r\n\x1bkec2...ReX\x1b\\\r\n' == '\nx'
FAILED tests/test_execution.py::test_execute_command_with_many_newlines_in_session - AssertionError: assert '\r\n\x1bkec2...ReX\x1b\\\r\n' == '\n\nx\n\n\n'
FAILED tests/test_execution.py::test_execute_command_with_whitespace_in_session - AssertionError: assert '\r\n\x1bkec2...ReX\x1b\\\r\n' == '  x'
FAILED tests/test_execution.py::test_execute_command_with_leading_space_in_session - AssertionError: assert '\r\n\x1bkec2...ReX\x1b\\\r\n' == '\n \nhello\nworld\n'
FAILED tests/test_execution.py::test_run_in_shell_timeout - swerex.exceptions.NoExitCodeError: timeout while getting exit code
FAILED tests/test_execution.py::test_run_in_shell_interactive_command_timeout - Failed: DID NOT RAISE <class 'swerex.exceptions.CommandTimeoutError'>
FAILED tests/test_execution.py::test_multiple_isolated_shells - AssertionError: assert '\r\n42\n\x1b...ReX\x1b\\\r\n' == '42\n'
FAILED tests/test_execution.py::test_multiple_commands_with_linebreaks_in_shell - AssertionError: assert '\r\n\x1bkec2...ReX\x1b\\\r\n' == 'test1\ntest2\n'
FAILED tests/test_execution.py::test_bash_multiline_command_eof - AssertionError: assert '\r\n\x1bkec2...ReX\x1b\\\r\n' == 'hello world\nhello world 2\n'
FAILED tests/test_execution.py::test_run_just_comment - AssertionError: assert '\r\n\x1bkec2...ReX\x1b\\\r\n' == ''
FAILED tests/test_execution.py::test_run_in_shell_multiple_commands - AssertionError: assert '\r\n\x1bkec2...ReX\x1b\\\r\n' == 'hello world\nhello again\n'
FAILED tests/test_execution.py::test_run_in_shell_while_loop - AssertionError: assert '\r\n\x1bkec2...ReX\x1b\\\r\n' == 'hello world\...hello world\n'
FAILED tests/test_execution.py::test_with_bashlex_errors - AssertionError: assert '\r\n\x1bkec2...ReX\x1b\\\r\n' == 'hw\nasdf\n'
FAILED tests/test_execution.py::test_fail_bashlex_errors - AssertionError: assert '\r\n\x1bkec2...ReX\x1b\\\r\n' == ''
FAILED tests/test_execution.py::test_echo_new_lines - AssertionError: assert '\r\n\x1bkec2...ReX\x1b\\\r\n' == 'hello\nworld\n'
FAILED tests/test_execution.py::test_interrupt_session - AssertionError: assert '\r\nasdf\n\x...ReX\x1b\\\r\n' == 'asdf\n'
FAILED tests/test_execution.py::test_interrupt_pager - swerex.exceptions.NoExitCodeError: failed to parse exit code from output '\r\n\x1b=\rblargh\n(END)\r\x07\r(END)\r\r(END)\x07\r(END)\r...skipping...\n\n                   SUMMARY OF LESS COMMANDS\n\n      Commands marked wi...
===================================================================================== 18 failed, 29 passed, 1 xfailed, 34 warnings in 23.86s ======================================================================================

I went ahead and pushed to this branch because it seemed easiest but can also pull my changes back out into my fork if that's better. Thanks! Let me know if there are any other issues.

CC: @joyliu-q

@saltzm
Copy link
Collaborator

saltzm commented Jul 2, 2025

Looks like tests all passed in CI, let me know if you have any other feedback 🙂

@joyliu-q joyliu-q changed the title (rebased #197 for testing) Make remote runtime fully async with aiohttp + testing Jul 14, 2025
@saltzm
Copy link
Collaborator

saltzm commented Aug 1, 2025

Hi! Just checking on this

@klieret klieret merged commit f50b5f5 into main Aug 6, 2025
5 checks passed
@klieret klieret deleted the rebased-async-remote-runtime branch August 6, 2025 00:58
@klieret
Copy link
Member Author

klieret commented Aug 6, 2025

Hi @saltzm thank you so much for the PR! I've just merged it :) Will look at the other PRs in a second.

Sorry again that this took for forever, but I'm really happy for the improvements!

@klieret klieret restored the rebased-async-remote-runtime branch August 6, 2025 01:04
@saltzm
Copy link
Collaborator

saltzm commented Aug 7, 2025

All good, thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants