-
Notifications
You must be signed in to change notification settings - Fork 56
Remove timeout loop in FluentConnection #2126
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
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.
This is a great idea, but the test_recover_grpc_error_from_launch_error
is going to leave unreachable zombies behind (watchdog also not going to work due to incorrect port number), and I have an idea of how to make this test work with containers as well, and clean up after itself so it doesn't leave zombies, going to see if I can make some suggestions
Edit: if you prefer, can merge this without the test for now, and then we can add more robust tests in a separate PR
Thanks @raph-luc, this is a good point. Feel free to merge your suggestions directly to this branch if you prefer. We'll complete the PR sometime next week. |
Thanks @raph-luc, I've kept both tests. Both are useful examples of how to recover the grpc error in those scenarios. |
* Remove timeout * Remove timeout * Fix test * Remove timeout argument * Call the correct health-check * Add test * Add test * Fix * Fix test * Disable test for docker * Add test_recover_grpc_error_from_connection_error
* Fix vale warnings (#2139) * fix tensor type for displacement variable (#2145) * Fix set_state implementation for command argument instance. (#2147) * Update flobject.py (#2148) * SVAR Doc (#1635) * Test to catch Watchdog launch errors, and improved Watchdog behavior on Windows (#2144) * Cavitation Model Example And Example Warning Fix (#2102) * Add type annotations for some modules under services (#2108) * More robust Windows launch command for Watchdog (#2167) * Making h5py an optional dependency, not installed by default (#2171) * Expose settings root like in pyconsole. (#2149) * Remove timeout loop in FluentConnection (#2126) * Fix SVAR doc (#2172) --------- Co-authored-by: Mainak Kundu <94432368+mkundu1@users.noreply.github.com> Co-authored-by: Oleg Chernukhin <92750311+ochernuk@users.noreply.github.com> Co-authored-by: Prithwish Mukherjee <109645853+prmukherj@users.noreply.github.com> Co-authored-by: Harshal Pohekar <106588300+hpohekar@users.noreply.github.com> Co-authored-by: Aseem Jain <95020968+ajain-work@users.noreply.github.com> Co-authored-by: Prithwish Mukherjee <prithwish.mukherjee@ansys.com> Co-authored-by: Adam Boutin <143635850+ansaboutin@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
trying to understand the impactLooking at how we are launching Fluent from PyFluent, the timeout loop in
FluentConnection
is unnecessary. I have replaced it with acheck_health
call which will fail only if there is some issue at grpc communication level and it will expose the grpc error message in that case. Exposing the grpc error message here will help us to triage issues in user's system.The
start_timeout
is used only when we wait for the server to finish writing the server-info file.Tested by hardcoding a port in
FluentConnection
, so grpc connection will fail due to port-mismatch:Old behaviour (fails after 60 s):
New behaviour (fails immediately after Fluent is launched and shows the grpc error message):