SSHLibrary
SSHLibrary copied to clipboard
robotframework
add read buffer to improve performance
Add a read buffer to improve performance of read_until and derived functions.
Should fix https://github.com/robotframework/SSHLibrary/issues/425 - I used the tests included in the issue; the execution time of Read Until and pure paramiko is comparable. When used in internal tests I did, it reduced execution time from minutes to seconds.
I just wanted to say: THANK YOU FOR HELPING DEVELOP THIS! This outright fixes performance issues when clients have a very long output, and is even noticeable in the general case.
Maintainers, please merge this PR ASAP!
Many thanks again to uruun, what a legend for helping with this fix
Just found out an issue that this will fail with UnicodeDecodeError - unexpected end of data. Will fix soon.
@rticau should I test it on local environment instead of CI? It seems it cannot find a Ubuntu 18 runner. I would post the results here.
@uruun We are trying to figure out why tests are not running. But meanwhile I guess it wouldn't hurt if you could run the tests yourself. Thanks!
Any update on this pull request? Seems like the CI jobs just need to pass and this can be merged, right?
@rticau , other folks: Can we have this merged? I'm happy to help provide testing info or debug the CI builds. This is a really fantastic improvement.
@scott-carrion Hi. This was not merged as the tests did not pass and I really do not have the time to check. If you can take a look, it would be great.
@rticau , sorry, I haven't had time to either. Is there a way to re-run the CI tests so I can look at the logs and debug?
@scott-carrion I think a small commit would re-trigger the tests (I didn't find an option to trigger manually).
Turns out there were some issues, I just pushed fixes, I hope now it passes :)
Some issues I can still see:
- The entire build job seems to be failing because robotstatuschecker 1.x uses deprecated functions from robotframework. It works after I installed robotstatuschecker >2.x but it dropped python 2 support. No idea if it will be an issue to use newer robotstatuschecker upstream.
- Some tests are failing because of paramiko (probably this https://github.com/paramiko/paramiko/issues/2048)
- Some tests are failing and it's expected but robotstatuschecker fails to parse them because it doesn't work at all
- No idea about Jython, I didn't test it when I submitted this PR and I don't know why it fails.
Hi,
sorry, I had to merge a big PR for dropping Python2 and Jython support - which also included major restructuring of the entire code. Could you maybe reintegrate your changes in to master? I hope it is just integrating the changes in to client.py .
Thanks
