haze518 commented on code in PR #1904:
URL: https://github.com/apache/iggy/pull/1904#discussion_r2163806038
##########
foreign/go/contracts/message_header.go:
##########
@@ -21,23 +21,21 @@ import (
"encoding/binary"
"errors"
"time"
-
- "github.com/google/uuid"
)
const MessageHeaderSize = 8 + 16 + 8 + 8 + 8 + 4 + 4
type MessageHeader struct {
- Checksum uint64 `json:"checksum"`
- Id uuid.UUID `json:"id"`
- Offset uint64 `json:"offset"`
- Timestamp uint64 `json:"timestamp"`
- OriginTimestamp uint64 `json:"origin_timestamp"`
- UserHeaderLength uint32 `json:"user_header_length"`
- PayloadLength uint32 `json:"payload_length"`
+ Checksum uint64 `json:"checksum"`
+ Id [16]byte `json:"id"`
Review Comment:
I don't really like that users will have to interact with a raw array. Maybe
it makes sense to add a type alias like
```go
type MessageID [16]byte
```
and immediately add a function for it, like
```go
func MessageIDFromUUID(u uuid.UUID) MessageID {
var id MessageID
copy(id[:], u[:])
return id
}
```
btw we could also add a FromBytes method for convenience, so it can be used
here
https://github.com/apache/iggy/pull/1904/files#diff-24507e9cc1ec9060e8f7e31afc8103078ed69dd7adad543e5cfe917698736c8bR53
It just seems like it would be more convenient this way. What do you think?
##########
foreign/go/contracts/messages.go:
##########
@@ -18,7 +18,12 @@
package iggcon
import (
- "github.com/google/uuid"
+ ierror "github.com/iggy-rs/iggy-go-client/errors"
+)
+
+const (
+ MaxPayloadSize = 10 * 1000 * 1000
+ MaxUserHeadersSize = 100 * 1000
Review Comment:
lets' add comments here, like in the rust sdk
##########
foreign/go/benchmarks/send_messages_benchmark_test.go:
##########
@@ -148,7 +148,11 @@ func CreateMessages(messagesCount, messageSize int)
[]iggcon.IggyMessage {
}
id, _ := uuid.NewUUID()
- messages[i] = iggcon.NewIggyMessage(id, payload)
+ var err error
Review Comment:
we can move the initialization of err up so that it doesn't get created
inside the loop
##########
foreign/go/e2e/tcp_test/messages_steps.go:
##########
@@ -37,11 +37,9 @@ func createDefaultMessageHeaders()
map[iggcon.HeaderKey]iggcon.HeaderValue {
func createDefaultMessages() []iggcon.IggyMessage {
headers := createDefaultMessageHeaders()
- messages := []iggcon.IggyMessage{
- iggcon.NewIggyMessageWithHeaders(uuid.New(),
[]byte(createRandomString(256)), headers),
- iggcon.NewIggyMessageWithHeaders(uuid.New(),
[]byte(createRandomString(256)), headers),
- }
-
+ msg1, _ := iggcon.NewIggyMessage([]byte(createRandomString(256)),
iggcon.WithID(uuid.New()), iggcon.WithUserHeaders(headers))
+ msg2, _ := iggcon.NewIggyMessage([]byte(createRandomString(256)),
iggcon.WithID(uuid.New()), iggcon.WithUserHeaders(headers))
+ messages := []iggcon.IggyMessage{msg1, msg2}
return messages
Review Comment:
You can return the slice directly here
```go
return []iggcon.IggyMessage{msg1, msg2}
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
