Closed LourensVeen closed 6 years ago
djgroen
commented
6 years ago Hi Lourens.
The code looks clean to me, the integration tests pass, and the quality review messages are all about unused variables and functions that could be decorated.
I think some of these code quality are due to us planning to add more functionality as we go, so I'm happy to accept this pull request.
LourensVeen
commented
6 years ago Actually, a few of them were actually real. Codacy does tend to give false positives fairly often, but you can disable rules (e.g. it warns against using assert, because some people use it incorrectly, but we're using pytest, which is designed to use plain assert, so now Codacy flags all the tests. So I disabled that rule.) and ignore specific cases, so it's still helpful.
I've fixed what could be fixed, disabled and ignored almost everything else, and I'm going to be so bold as to merge that into develop myself :).
This pull request adds a first simple gRPC protocol, tooling around that, a simple server inside the manager, a simple client inside libmuscle, unit tests for both, and an integration test that tests them together.
It turns out that gRPC doesn't generate normal Python 3 classes, but has some reflection-based thing that's a bit different from normal Python classes. It doesn't support Python 3 enums either (being ten years old, that's reasonable I guess, but annoying). So I've written wrapper classes. That's not ideal to do by hand, but like this the gRPC objects are confined to a small bit of the code, and the rest is easy to read and understand.
I've added the gRPC-generated files to the repository, rather than rebuilding them on install (rebuilding is easy though, just use
make grpc). Adding generated files to a repository is generally speaking not a great idea, but I expect the protocol to stabilise pretty quickly. If you change the .proto file, you'll need to change the wrapper classes as well anyway, and test it, so you might as well commit the whole thing. And Python is an interpreted language, so having to build things before you can run is somewhat counter-intuitive.At the moment, some of the code looks like boilerplate, but that will get better as we add more functionality.