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 2020/04/15 10:08:14 UTC
[GitHub] [pulsar-client-go] skyrocknroll opened a new pull request #225:
Added an function to serialize the messageID to string
skyrocknroll opened a new pull request #225: Added an function to serialize the messageID to string
URL: https://github.com/apache/pulsar-client-go/pull/225
MessageID to String Serialization
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar-client-go] skyrocknroll commented on issue #225: Added an
function to serialize the messageID to string
Posted by GitBox <gi...@apache.org>.
skyrocknroll commented on issue #225: Added an function to serialize the messageID to string
URL: https://github.com/apache/pulsar-client-go/pull/225#issuecomment-614073204
@merlimat Can you please review it?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar-client-go] skyrocknroll closed pull request #225: Added an
function to serialize the messageID to string
Posted by GitBox <gi...@apache.org>.
skyrocknroll closed pull request #225: Added an function to serialize the messageID to string
URL: https://github.com/apache/pulsar-client-go/pull/225
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar-client-go] skyrocknroll commented on a change in pull
request #225: Added an function to serialize the messageID to string
Posted by GitBox <gi...@apache.org>.
skyrocknroll commented on a change in pull request #225: Added an function to serialize the messageID to string
URL: https://github.com/apache/pulsar-client-go/pull/225#discussion_r408936946
##########
File path: pulsar/impl_message.go
##########
@@ -93,6 +94,10 @@ func (id *messageID) Serialize() []byte {
data, _ := proto.Marshal(msgID)
return data
}
+func (id *messageID) SerializeToString() string {
+
+ return fmt.Sprintf("%d:%d:%d:%d", id.ledgerID, id.entryID, id.batchIdx, id.partitionIdx)
Review comment:
Make sense. Thanks @merlimat
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar-client-go] merlimat commented on a change in pull request
#225: Added an function to serialize the messageID to string
Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #225: Added an function to serialize the messageID to string
URL: https://github.com/apache/pulsar-client-go/pull/225#discussion_r408900715
##########
File path: pulsar/impl_message.go
##########
@@ -93,6 +94,10 @@ func (id *messageID) Serialize() []byte {
data, _ := proto.Marshal(msgID)
return data
}
+func (id *messageID) SerializeToString() string {
+
+ return fmt.Sprintf("%d:%d:%d:%d", id.ledgerID, id.entryID, id.batchIdx, id.partitionIdx)
Review comment:
My concern here is that we're exposing the String format in the API directly. The string format itself shouldn't be used for any other purpose than debug, since it's not guaranteed to be kept compatible.
For that, if the purpose of this is to print a message id on logs, wouldn't that already work even without the `SerializeToString()`?
For example, check https://github.com/apache/pulsar-client-go/blob/master/examples/consumer/consumer.go#L52-L53 There, all the internal components of the message id are going to get printed on the log.
----------------------------------------------------------------
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
With regards,
Apache Git Services