Conversation
Signed-off-by: Jordan Evans <jevans@linuxfoundation.org>
There was a problem hiding this comment.
Pull request overview
This PR removes MessagePack (msgpack) encoding support from DynamoDB stream ingestion, standardizing on JSON-only encoding for this specific ingestion path. The change is temporary ("for now" per the title) and affects how DynamoDB stream events are stored in the NATS KV bucket.
Changes:
- Removed msgpack import from ingest_dynamodb.go
- Removed msgpack fallback when unmarshaling existing KV entries (previously tried msgpack if JSON failed)
- Removed conditional encoding logic that respected cfg.UseMsgpack, now always uses JSON for marshaling
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if unmarshalErr := json.Unmarshal(existing.Value(), &existingData); unmarshalErr != nil { | ||
| if msgpackErr := msgpack.Unmarshal(existing.Value(), &existingData); msgpackErr != nil { | ||
| logger.With(errKey, unmarshalErr, "msgpack_error", msgpackErr, "key", key). | ||
| ErrorContext(ctx, "failed to unmarshal existing KV entry") | ||
| logger.With(errKey, unmarshalErr, "key", key).ErrorContext(ctx, "failed to unmarshal existing KV entry") | ||
| return false | ||
| } | ||
| } |
There was a problem hiding this comment.
The code now always uses JSON encoding for DynamoDB events. However, if any existing KV entries were previously stored in msgpack format (when cfg.UseMsgpack was true), this code will now fail to unmarshal them since the msgpack fallback has been removed. Consider documenting this breaking change or adding a migration path if there's existing msgpack-encoded data in the v1-objects KV bucket from DynamoDB sources.
|
This can be closed in favor of #42 |
3 participants
No description provided.