Adding Pinger interface and Removing mandatory nature of Ping fuction #1784
Conversation
|
Please fix DCO. |
bindings/input_binding.go
Outdated
| if inputBindingWithPing, ok := inputBinding.(health.Pinger); ok { | ||
| return inputBindingWithPing.Ping() | ||
| } else { | ||
| return fmt.Errorf("Ping is not implemented by this Input Binding") |
There was a problem hiding this comment.
| return fmt.Errorf("Ping is not implemented by this Input Binding") | |
| return fmt.Errorf("Ping is not implemented by this input binding") |
this should be lowercase for consistency and it currently breaks tests
There was a problem hiding this comment.
Done. To understand that what tests are failing and how these rules are setup, I tried to search, are these the Conformance tests that are failing? I could not find this in UTs.
There was a problem hiding this comment.
Ok, so now almost all Conf tests started to pass after this. I would need to check how this is checked in conf tests.
There was a problem hiding this comment.
You had inconsistency in spelling between your conformance tests and your error implementation if you look at your PR carefully :)
At some place you would essentially raise error "Error" but somewhere else expect error "error" if that makes sense.
Signed-off-by: chanyong.moon <dev.chanyongmoon@gmail.com>
Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>
Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>
DeepanshuA
force-pushed
the
cc_componentsHealth
branch
from
f57198d
to
123356a
Compare
Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #1784 +/- ##
==========================================
+ Coverage 36.59% 36.64% +0.04%
==========================================
Files 177 179 +2
Lines 16222 16347 +125
==========================================
+ Hits 5937 5990 +53
- Misses 9617 9671 +54
- Partials 668 686 +18
Continue to review full report at Codecov.
|
DeepanshuA
changed the title
DeepanshuA
changed the title
…dapr#1784) * add `Pinger` interface. Signed-off-by: chanyong.moon <dev.chanyongmoon@gmail.com> * Convert Ping fuction to optional Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com> * Removing unrequired Ping implementations Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com> * Addressing comments Signed-off-by: Deepanshu Agarwal <deepanshu.agarwal1984@gmail.com> Co-authored-by: chanyong.moon <dev.chanyongmoon@gmail.com>
Description
This change introduces Pinger interface and also reduces mandatory Ping functionality.
Ping works on the basis, that wen called, it returns either an Error or nil.
With forced implementation, any component returning without checking actual heartbeat and simply returning nil, causes wrong sense of correctness of component. So, ideally Ping should be implemented only when correctly implemented.
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: dapr/dapr#2167
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: