flower icon indicating copy to clipboard operation
flower copied to clipboard

VCE does not handle missing `get_properties`

Open danieljanes opened this issue 3 years ago • 2 comments

Describe the bug

Classes derived from Client and NumPyClient can decide whether they want to override get_properties or not, so get_properties is optional.

When using the gRPC-based Edge Client Engine (ECE), both start_client and start_numpy_client handle this optionality. See, for example, the message handler: https://github.com/adap/flower/blob/14b44a827cffa4b7128880ba729d7c53f6c64a33/src/py/flwr/client/grpc_client/message_handler.py#L85

When using the Ray-based Virtual Client Engine (VCE), the optionality is not handled: https://github.com/adap/flower/blob/14b44a827cffa4b7128880ba729d7c53f6c64a33/src/py/flwr/simulation/ray_transport/ray_client_proxy.py#L93

Steps/Code to Reproduce

Define a subclass of Client that does not implement get_properties and use is together with start_simulation.

Expected Results

The PropertiesRes message returned from Client.get_properties has a Status field that can indicate that get_properties is not implemented by the user-provided Client instance: https://github.com/adap/flower/blob/14b44a827cffa4b7128880ba729d7c53f6c64a33/src/py/flwr/client/grpc_client/message_handler.py#L89

When using get_properties with start_simulation, PropertiesRes should contain the Status with the appropriate code and message set (depending on whether get_properties was implemented or not).

Actual Results

start_simulation does not check whether get_properties is implemented by the user-provided Client instance and does not set Status accordingly.

danieljanes avatar Mar 28 '22 16:03 danieljanes

I installed flower using pip and it's throwing the AttributeError: get_properties when I try to run the client. Is the bug solved? If yes, how shall I proceed?

tusharvatsa32 avatar Apr 02 '22 17:04 tusharvatsa32

@tusharvatsa32 this should be resolved with the recent release of Flower 1.0, can you confirm?

danieljanes avatar Aug 10 '22 21:08 danieljanes