You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/05/22 21:54:35 UTC
[GitHub] [pulsar-client-go] addisonj opened a new pull request #526: Add message string
addisonj opened a new pull request #526:
URL: https://github.com/apache/pulsar-client-go/pull/526
If we want to deal with string message ids, we need methods for being
able to create them from either a string or a using the individual parts
of the message id.
This exposes some constructors for this as well as an equals method to make testing more straight forward (but I am not sure if we want to expose equals...)
This commit adds tests for the needed new methods
--
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:
users@infra.apache.org
[GitHub] [pulsar-client-go] cckellogg commented on a change in pull request #526: Add messageID from string and from parts
Posted by GitBox <gi...@apache.org>.
cckellogg commented on a change in pull request #526:
URL: https://github.com/apache/pulsar-client-go/pull/526#discussion_r638085123
##########
File path: pulsar/message.go
##########
@@ -120,19 +124,65 @@ type Message interface {
type MessageID interface {
// Serialize the message id into a sequence of bytes that can be stored somewhere else
Serialize() []byte
+ // String the message id represented as a string
+ String() string
+ // Equals indicates to message IDs are equal
+ Equals(other MessageID) bool
Review comment:
I don't think we should add this either. We should avoid changing the interfaces since it's a breaking change and this can be accomplished with out doing that. We can add a util method like `messageIDsEqual or MessageIDsEqual`.
--
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:
users@infra.apache.org
[GitHub] [pulsar-client-go] wolfstudy commented on pull request #526: Add messageID from string and from parts
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #526:
URL: https://github.com/apache/pulsar-client-go/pull/526#issuecomment-849473142
ping @addisonj any update for this?
--
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:
users@infra.apache.org
[GitHub] [pulsar-client-go] cckellogg commented on a change in pull request #526: Add messageID from string and from parts
Posted by GitBox <gi...@apache.org>.
cckellogg commented on a change in pull request #526:
URL: https://github.com/apache/pulsar-client-go/pull/526#discussion_r638077068
##########
File path: pulsar/message.go
##########
@@ -120,19 +124,65 @@ type Message interface {
type MessageID interface {
// Serialize the message id into a sequence of bytes that can be stored somewhere else
Serialize() []byte
+ // String the message id represented as a string
+ String() string
Review comment:
This is not needed the struct just needs to implement the Stringer interface
https://golang.org/pkg/fmt/#Stringer
##########
File path: pulsar/message.go
##########
@@ -120,19 +124,65 @@ type Message interface {
type MessageID interface {
// Serialize the message id into a sequence of bytes that can be stored somewhere else
Serialize() []byte
+ // String the message id represented as a string
+ String() string
+ // Equals indicates to message IDs are equal
+ Equals(other MessageID) bool
Review comment:
I don't think we should add this either. We should avoid changing the interfaces since it's a breaking change and this can be accomplished with out doing that. We can add a util method like `messageIDsEqual or MessageIDsEquals`.
--
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:
users@infra.apache.org
[GitHub] [pulsar-client-go] wolfstudy commented on a change in pull request #526: Add messageID from string and from parts
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on a change in pull request #526:
URL: https://github.com/apache/pulsar-client-go/pull/526#discussion_r637672167
##########
File path: pulsar/impl_message.go
##########
@@ -112,6 +112,14 @@ func (id messageID) String() string {
return fmt.Sprintf("%d:%d:%d", id.ledgerID, id.entryID, id.partitionIdx)
}
+func (id messageID) Equals(other MessageID) bool {
+ rmsgid, ok := other.(messageID)
+ if !ok {
+ return false
+ }
+ return id.equal(rmsgid)
+}
+
Review comment:
The `MessageID` is an interface, Are we here trying to compare whether two interfaces are equal?
--
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:
users@infra.apache.org