georgymh / decentralized-ml

Interoperable and decentralized machine learning.
Apache License 2.0
9 stars 5 forks source link

Add bootstrapper with flask-based comm. manager & a few clean ups #8

Closed georgymh closed 6 years ago

georgymh commented 6 years ago

This pull request's major change is the addition of the Bootstrapper (see the UNIX Service wiki on Quip for more information).

In details, these are all the changes:

neeleshdodda44 commented 6 years ago

Are there going to be tests for the Bootstrapper/Communication Manager/Listener?

Seems kind of nitpicky for Communication Manager/Listener (tests for components used for testing), but I was just wondering.

georgymh commented 6 years ago

I think writing tests should be prioritized for modules that would go under considerable changes over their lifetime.

So the Communication Manager and Listener (which will be embedded in the Communication Manager) should definitely have tests.

As for this Listener, I decided not to write tests since it's only used to manually debug the service and we don't expect to be changing it much. It's not a critical component.

The tests for the Bootstrapper will probably be integration tests, but we need to finish the other modules before writing those. I don't think unit tests make a lot of sense for the Bootstrapper right now.

georgymh commented 6 years ago

Perfect, thanks @neeleshdodda44.

@nzoghb, I'm waiting on your review for the multithreaded & multiprocessing parts.

georgymh commented 6 years ago

@nzoghb: Yes, those few bits. If you don't see any problem with them, I will go ahead and merge. I fixed the test & private method issue and left a comment above.