nitishv commented on a change in pull request #222:
URL: https://github.com/apache/pulsar-client-go/pull/222#discussion_r449955387
##########
File path: pulsar/reader_test.go
##########
@@ -362,3 +365,108 @@ func TestReaderHasNext(t *testing.T) {
assert.Equal(t, 10, i)
}
+
+func TestReaderSeek(t *testing.T) {
Review comment:
There can be an added test case which does a Seek on a custom MessageID
refer to https://github.com/apache/pulsar-client-go/pull/305
##########
File path: pulsar/reader_impl.go
##########
@@ -150,3 +151,33 @@ func (r *reader) hasMoreMessages() bool {
func (r *reader) Close() {
r.pc.Close()
}
+
+func (r *reader) messageID(msgID MessageID) (*messageID, bool) {
+ mid, ok := msgID.(*messageID)
+ if !ok {
+ r.log.Warnf("invalid message id type")
+ return nil, false
+ }
+
+ partition := mid.partitionIdx
+ // did we receive a valid partition index?
+ if partition != 0 {
+ r.log.Warnf("invalid partition index %d expected 0", partition)
+ return nil, false
+ }
+
+ return mid, true
+}
Review comment:
Given the method name, the purpose of this method is to convert
MessageID to get *messageID
In doing so failure in type assertion is not really a failure. The given
msgID can still be converted. Refer to a this fix I just made
https://github.com/apache/pulsar-client-go/pull/305
I think that change can be re-factored into this unexported method for
reader, as it being used in couple of places now with this change.
Also checking for non-zero partition index does not seem to be the purpose
of this method, and should not be included here. That check is only relevant to
Seek APIs, and should be moved there.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
