Make debug easier #854
Make debug easier #854
Conversation
|
Hi @dtaniwaki. Thanks for your PR. I'm waiting for a SeldonIO member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Are we sure this covers cases discussed in #700 where the user has not extended the Seldon class but simply created their own so you would get a AttributeError? |
| @@ -78,16 +78,18 @@ def send_feedback(user_model: Any, request: prediction_pb2.Feedback, | |||
| else: | |||
| try: | |||
| return user_model.send_feedback_raw(request) | |||
| except (NotImplementedError, AttributeError): | |||
There was a problem hiding this comment.
How do AttributeErrors can handled now? Can we add a test
There was a problem hiding this comment.
Do we need to rescue AttributeError? send_feedback_raw originally raises NotImplementedError.
There was a problem hiding this comment.
I got your point. I'll add has_attr check. Can I also include the error class change for #700?
There was a problem hiding this comment.
Yes I added Fix in your top level comment. So yes if you could extend to cover #700
|
Hi @cliveseldon I updated the code. Could you check it again? |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cliveseldon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/ok-to-test |
Currently, some types of errors occurred in models are swallowed and it's really hard to debug because the server doesn't show any log and we don't know if an error occurred or the response is really empty.
To make debugging easier, I made the following changes in this PR.
AttributeErroras the methods should not raise this error.NotImplementedError.Fixes #700