search

src-d / lookout-sdk

SDK for lookout analyzers
Apache License 2.0
4 stars 11 forks source link

[Python SDK] Adds gRPC logger interceptors #80

Closed se7entyse7en closed 5 years ago

se7entyse7en commented 5 years ago

Part of #71.

This depends on #78.

smacker commented 5 years ago

Oh. Sorry. I approved wrong PR. I still need to review this one.

smacker commented 5 years ago

I took a quick look but I'll do a proper review when #78 is merged because this one depends on it.

se7entyse7en commented 5 years ago

I missed to write a proper description, but I think that the docstrings are well detailed. For the review I suggest you to take a look at https://github.com/src-d/lookout-sdk/blob/5abdaaa81ca951d71e3e03e31a6adef2eaeb60d5/python/lookout/sdk/grpc/interceptors/utils.py first, especially to read the docstring.

se7entyse7en commented 5 years ago

@smacker I've just realized that something went wrong during the rebase and the interceptor is never bound 😕. We should also create an issue to add some tests.

smacker commented 5 years ago

python interceptors? Let's add tests here for python.

se7entyse7en commented 5 years ago

@smacker I've added tests for the python SDK and a corresponding make target called test-python-sdk:

Screenshot 2019-03-13 at 23 33 43

For now, I haven't added any coverage threshold, but it just prints a report. I've also added a couple of fixes that I discovered during testing. You can easily find them by reading the message of the new commits.