google / avatar

Apache License 2.0
20 stars 10 forks source link

Add timeout to all ConnectLE() and Scan() methods #55

Closed zxzxwu closed 1 year ago

zxzxwu commented 1 year ago

Current ConnectLE() and Scan() methods miss a native timeout condition, so timeout must be set explicitly to avoid halting the tests.

uael commented 1 year ago

We need something that works for non-async tests as well, using a thread or a signal ? :thinking:

uael commented 1 year ago

Or we can add something like this in the setup_test ?

import signal

def setup_test(self) -> None:
    signal.signal(signal.SIGALRM, lambda _signum, _frame: raise TimeoutError("test timed out"))
    signal.alarm(30)
DeltaEvo commented 1 year ago

Could we instead set a default deadline on GRPC calls ?

zxzxwu commented 1 year ago

Could we instead set a default deadline on GRPC calls ?

It's also a solution, but I didn't find any global timeout/deadline configuration. Or we may manually add timeout to each call, and setup some lint there.

uael commented 1 year ago

We can also change the gRPC code generator, but it's a radical solution

uael commented 1 year ago

And on server side ?

zxzxwu commented 1 year ago

And on server side ?

The safest way is to do on the both side and select the shorter one, but if we must pick one, client side is safer because

zxzxwu commented 1 year ago

Rebase on https://github.com/google/avatar/pull/58

uael commented 1 year ago

WDYT on using literals directly without any forwarded constant ?

zxzxwu commented 1 year ago

WDYT on using literals directly without any forwarded constant ?

That's a good idea. I moved them into an user param.