Skip to content
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

Refactoring of StdoutProxy to take app_session from the caller #1854

Closed

Conversation

jooncheol
Copy link

This changes makes StdoutProxy to take the exact app_session from by taking it as optional argument in the constructor instead finding it by calling get_app_session() globally.

It can avoid confliction of printing output within the patch_stdout context in the ssh session when the multiple ssh connections are performing concurrently.

The changed StdoutProxy now passes the correct app instance from the given app_session in the constructor to the run_in_terminal() in it instead of calling get_app_or_none() globally that can give wrong app instance from the prompt session in the last ssh connection.

The key usage has been added into the example ssh/asyncssh-server.py.


How to test

  • run asyncssh-server.py
  • open two more terminals and run "ssh -p 8222 localhost"
% ssh -p 8222 localhost
We will be running a few prompt_toolkit applications through this 
SSH connection.

 100.0% [================================================>]  50/ 50 eta [00:00]
(normal prompt) Type something:
You typed 
(autocompletion) Type an animal:
You typed 
(HTML syntax highlighting) Type something:
You typed 
Showing yes/no dialog... [ENTER]
Showing input dialog... [ENTER]
Counter: 0
Counter: 1
Counter: 2
Counter: 3
Type something with background task: test
You typed: test
Connection to localhost closed.
jooncheol@Jooncheols-MBP ~ % 

Result

  • both of ssh client connection shows "Counter: 0 ~ N" and keep protect the prompt string "Type something with background task:" from the print attempting in the background task.

Background & motivation
I found the issue when I used patch_stdout in the asyncssh-server.py based custom ssh server.
Basically the print within the patch_stdout block was messed because of the multiple the patch_stdout context interfere the other one from the other ssh session.

I used "with StdoutProxy(...) as output" in the example code asyncssh-server.py to test this patch. It can more explicitly use the right output object than using "print" within "patch_stdout" block in case of the ssh server example.
I wanted to touch "patch_stdout" to take the app_session too to achieve the goal, but It seems to be dangerous for the existing echo system of this project. so, I tried to modify only the StdoutProxy class to take the additional optional parameter 'app_session'.

Thanks in advance for the feedback / review. and thanks for the good library!

@jooncheol jooncheol force-pushed the patch_output_for_ssh branch 5 times, most recently from c0748d3 to 0bb55c8 Compare February 28, 2024 22:09
This changes makes StdoutProxy to take the exact app_session from
by taking it as optional argument in the constructor instead finding
it by calling get_app_session() globally.

It can avoid confliction of printing output within the patch_stdout
context in the ssh session when the multiple ssh connections are
performing concurrently.

The changed StdoutProxy now passes the correct app instance from the
given app_session in the constructor to the run_in_terminal() in it
instead of calling get_app_or_none() globally that can give wrong
app instance from the prompt session in the last ssh connection.
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.

None yet

1 participant