flower icon indicating copy to clipboard operation
flower copied to clipboard

feat(framework) Implement `GrpcRereFleetConnection`, `GrpcBidiConnection`, `GrpcAdapterFleetConnection`, and `RestFleetConnection`

Open panh99 opened this issue 1 year ago • 1 comments

Based on:

  1. #4054
  2. #4055
  • Migrated grpc_rere_client/connection.py to RereFleetConnection, implementing Connection class, and GrpcRereFleetConnection, implementing RereFleetConnection class.
  • Migrated part of rest_client/connection.py to RestFleetConnection. It now inherits from RereFleetConnection and thus the code duplication is minimized.
  • Migrated grpc_adapter_client/connection.py to GrpcAdapterFleetConnection, which also inherits from RereFleetConnection.
  • Migrated grpc_client/connection.py (the grpc-bidi transport) to GrpcBidiConnection, implementing Connection class.
  • Simplified the logic in the main loop of start_client_internal.
  • Updated client_interceptor.py and its unit tests accordingly.

panh99 avatar Aug 22 '24 09:08 panh99

Based on:

  1. feat(framework) Add the abstract base class Connection #4054
  2. refactor(framework:skip) Move existing transport code to flwr/client/connection folder #4055
  • Migrated grpc_rere_client/connection.py to GrpcRereConnection, implementing Connection class.
  • Migrated part of rest_client/connection.py to RestConnection. It now inherits from GrpcRereConnection and thus the code duplication is minimized.
  • Migrated grpc_adapter_client/connection.py to GrpcAdapterConnection, which also inherits from GrpcRereConnection.
  • Migrated grpc_client/connection.py (the grpc-bidi transport) to GrpcBidiConnection, implementing Connection class.
  • Simplified the logic in the main loop of start_client_internal.
  • Updated client_interceptor.py and its unit tests accordingly.

@panh99 Would it make sense to have an intermediary RereConnection that is inherited by both RestConnection and GrpcRereConnection? It somehow feels wrong to have RestConnection inherit from GrpcRereConnection eventhough it has nothing to do with gRPC. It's just a naming issue, but wdyt?

charlesbvll avatar Oct 09 '24 07:10 charlesbvll