Expose gRPC parameters in start_client() and start_numpy_client()

[g-pichler]

Arguments of the call to class grpc.StreamStreamMultiCallable can be passed in grpc_args and grpc_kwargs. These are timeout, metadata, credentials, wait_for_ready and compression (https://grpc.github.io/grpc/python/grpc.html?highlight=wait_for_ready#grpc.StreamStreamMultiCallable).

What does this implement/fix? Explain your changes.

This change exposes the parameters for gRPC (used by flower under the hood) to the user. I use this to set wait_for_ready=True to ensure that a client waits until a server is up.

[orlandohohmeier]

Love the introduction of gRPC wait_for_ready to improve connection robustness, but would change the interface to not leak the client abstraction.

[danieljanes]

Thanks for the PR @g-pichler !

Would there be any downsides to generally setting wait_for_ready=True @g-pichler @orlandohohmeier @tanertopal and not exposing the flag to the user?

[danieljanes]

The current PR looks good to me though, I wouldn't mind exposing this to the user if it provides value.

[orlandohohmeier]

@danieljanes the client currently fails if a connection to the server cannot be established. With wait_for_ready enabled the client would wait for the server till the RPC’s deadline is reached breaking with the current behavior.

[orlandohohmeier]

I wouldn't mind exposing this to the user if it provides value.

@danieljanes Not sure what you're alluding to? Exposing the wait_for_ready option?

[g-pichler]

I think that in general users expect a client to terminate if a connection to the server cannot be established. Setting wait_for_ready=True breaks this assumption and current behavior. I think wait_for_ready=True by default is a bad idea. For example, if the client port is misconfigured, this behavior could be very frustrating.

I am currently dealing with a situation where I need to spawn many clients and servers in separate threads to test many runs of a federated learning system. Typically the client starts quicker than the server process. So, exposing this flag provides a lot of value.

[danieljanes]

I wouldn't mind exposing this to the user if it provides value.

@danieljanes Not sure what you're alluding to? Exposing the wait_for_ready option?

@orlandohohmeier 3761f5e changed the user-facing API to expose wait_for_ready to the user.

@g-pichler thanks for the context, makes sense

[g-pichler]

To follow up: Are the more changes necessary or are you planning to merge this PR?