Make the client's "start" endpoint

[lotif]

PR Type

Feature

Short Description

Clickup Ticket(s): https://app.clickup.com/t/8687c8ked

Making an endpoint to start a client. Some additional work was required:

• I had to move the MNIST client and model into the API (instead of it being a test utils)
• Changed RedisMetricsReporter to lazy load the Redis connection so it would work with mutiprocessing
• Changed existing tests accordingly, and also moved the test_metrics.py which was located in the wrong spot

If you want to test it yourself, you can do the following:

Start a server in the terminal:

$ python
Python 3.9.6 (default, Nov 10 2023, 13:38:27) 
[Clang 15.0.0 (clang-1500.1.0.2.5)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from functools import partial
>>> from florist.api.launchers.local import launch_server
>>> from florist.tests.integration.api.launchers.test_launch import get_server
>>> launch_server(partial(get_server, n_clients=1), "localhost:8080", 2, "logs/server.out")

Start client's Redis:

docker run --name redis-florist-client -d -p 6380:6379 redis:7.2.4 redis-server --save 60 1 --loglevel warning

Start the client service with the command below:

uvicorn florist.api.client:app --reload --port 8001

Then, start a client by making a GET request to the following URL (replace <local_path> with any local path in your machine):

http://localhost:8001/api/client/start?server_address=localhost:8080&client=MNIST&data_path=<local_path>&redis_host=localhost&redis_port=6380

You should receive a UUID as a response. Training should start and finish successfully, logs will be in the logs folder and you can pull the client's metrics from Redis by using the UUID as the key.

Tests Added

• Unit tests for the start endpoint
• New unit tests for the change in functionality in the RedisMetricsReporter
• Integration tests to come in a follow up PR

[jewelltaylor]

Curious why we have to add the type: ignore in when we previously did not have to have one. Is it because you made the return type of get_dataloaders more specific?

[lotif]

It's because before it was in the testing folder. The static code checking for tests is mostly disabled because of mocking and other code practices that are OK to do in testing but not in code that runs in prod.

[jewelltaylor]

Is it best to leave all definitions besides endpoints in seperate files and import? Or does it not really matter. I don't have a lot of experience with writing APIs so if this is an unnecessary nit, feel free to ignore

[lotif]

That makes sense, will move.

[jewelltaylor]

This is beyond the scope of this PR, but I am curious if you think we should explore how to dump metrics to redis at more frequent intervals. If we only do so at the end, then in the case of a crash we lose the metrics. Do you think this is something worthwhile to explore or not really important enough as of now to be thinking about?

[lotif]

I did it in this class, I added a call to dump at the end of each method so it will update redis every time a new metric is recorded. I think this kind of behaviour overkill for the main class in FL4Health but it's necessary here and easy enough to instrument.

[jewelltaylor]

good call, I like the distinction between local and "distributed" launchers that we will build out down the line

[jewelltaylor]

The PR looks good to me! I have ran the example you indicated in the PR with no issues and the tests are passing on my machine. I have left a few comments to get some clarification on some things for my personal understanding, but nothing I expect to change the code dramatically so I will approve but check back to see what you have to say to some of the queries.

In the longer term, I am excited to work with you to figure out how to generalize the clients so they can be more configurable and not have to rely on pre defining clients for each dataset. But I think for now this is great and allows us to start building out FLorist without sinking too much time in trying to solve the configuration problem from the start.

[lotif]

@jewelltaylor yes, I'm looking forward to getting to that point too! Just curious to know how we are gonna tackle that in the end. I'm with the same thinking, we have some other foundational work to do and that can be figured out later. I expect the code to move around and change a lot especially now that we are at the beginning.