-
Notifications
You must be signed in to change notification settings - Fork 10k
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
server tests : more pythonic process management; fix bare except:
#6146
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably better code but the build failed... maybe you must test it before
I was able to run the tests locally after making the connect routine match what the server does - it uses the first address family returned by getaddrinfo, whether it is ipv4 or ipv6. Otherwise, the connection failed because the server was listening on see also #5372 (comment) |
There is a hack on windows server test to force the server to listen 0.0.0.0. But it's probably not the same issue you faced with ipv6 |
…w repeat penalties default changed in (#6127)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into this, your contribution on server tests is highly appreciated 👍
If you don't mind I will request your review on changes related to python server tests.
I have included fixes for:
All tests are now working and the python code is clearer, great!
…gerganov#6146) * server tests : remove seemingly redundant newlines in print() * server tests : use built-in subprocess features, not os.kill and psutil * server tests : do not catch e.g. SystemExit; use print_exc * server tests: handle TimeoutExpired exception * server tests: fix connect on dual-stack systems * server: tests: add new tokens regex on windows generated following new repeat penalties default changed in (ggerganov#6127) * server: tests: remove the hack on windows since now we get the good socket family * server: tests: add new tokens regex following new repeat penalties default changed in (ggerganov#6127) * server: tests: add new tokens regex following new repeat penalties default changed in (ggerganov#6127) --------- Co-authored-by: Pierrick HYMBERT <[email protected]>
…gerganov#6146) * server tests : remove seemingly redundant newlines in print() * server tests : use built-in subprocess features, not os.kill and psutil * server tests : do not catch e.g. SystemExit; use print_exc * server tests: handle TimeoutExpired exception * server tests: fix connect on dual-stack systems * server: tests: add new tokens regex on windows generated following new repeat penalties default changed in (ggerganov#6127) * server: tests: remove the hack on windows since now we get the good socket family * server: tests: add new tokens regex following new repeat penalties default changed in (ggerganov#6127) * server: tests: add new tokens regex following new repeat penalties default changed in (ggerganov#6127) --------- Co-authored-by: Pierrick HYMBERT <[email protected]>
…gerganov#6146) * server tests : remove seemingly redundant newlines in print() * server tests : use built-in subprocess features, not os.kill and psutil * server tests : do not catch e.g. SystemExit; use print_exc * server tests: handle TimeoutExpired exception * server tests: fix connect on dual-stack systems * server: tests: add new tokens regex on windows generated following new repeat penalties default changed in (ggerganov#6127) * server: tests: remove the hack on windows since now we get the good socket family * server: tests: add new tokens regex following new repeat penalties default changed in (ggerganov#6127) * server: tests: add new tokens regex following new repeat penalties default changed in (ggerganov#6127) --------- Co-authored-by: Pierrick HYMBERT <[email protected]>
ref #6098 (comment)
This code has been consistently confusing to me. Here I suggest a few simplifications:
\n
to a random selection ofprint()
s. A habit from C perhaps? Let me know if any (or all) of these are intentional - it just looked strange to me to have a blank line after so many prints.except:
clause - it's wrong to ignore exceptions like KeyboardInterrupt and SystemExit, so I changed it toexcept Exception:
. Andtraceback.print_exc
seemed more appropriate here.