carletes / mock-ssh-server

Python mock SSH server for testing purposes
MIT License
56 stars 32 forks source link

close pipe to the standard stream after finishing #27

Closed njzjz closed 2 years ago

njzjz commented 2 years ago

This commit closes Popen after it send_exit_status. Otherwise, the standard stream will be unclosed, and it may hang when calling handle_client again.

swiatek25 commented 2 years ago

Hi @njzjz. I triggered test execution for your PR but seems we are running into some issues. I do not see that to be directly related with your change but we need to check that. Can you take a look at python2.7 checks.

If we cannot resolve that issue I think we think about dropping python 2.7 support.

njzjz commented 2 years ago

It should be related to my change - context manager support is added in Python 3.2 per documentation, i.e. this method. Is it ok to drop python 2.7 support?

swiatek25 commented 2 years ago

Hi, context manager contract in subprocess module you mean. Let's gather feedback on dropping python 2.7 support from the author of the project.

@carletes do you have an opinion on dropping python2.7 support?

@njzjz I guess try, finally replacement could work for now as well if you need quicker merge.

carletes commented 2 years ago

@carletes do you have an opinion on dropping python2.7 support?

Let's drop Python 2.7 support --- about time :)

carletes commented 2 years ago

I'm working on dropping Python 2.7 support now.

swiatek25 commented 2 years ago

@carletes do you have an opinion on dropping python2.7 support?

Let's drop Python 2.7 support --- about time :)

:rocket:

carletes commented 2 years ago

@njzjz: I just dropped Python 2.7 support in branch master. If you rebase your branch off it, your tests should pass now, I suppose.

njzjz commented 2 years ago

I've merged my branch from master.

swiatek25 commented 2 years ago

Great. All checks are fine. We are good to go.