search

cloudevents / sdk-go

Go SDK for CloudEvents
https://cloudevents.github.io/sdk-go/
Apache License 2.0
773 stars 214 forks source link

fix: support multiple amqp data fields #1034

Closed embano1 closed 1 month ago

embano1 commented 1 month ago

Support multidimensional data field in AMQP implementation when converting an AMQP message to a CloudEvent. Writing to AMQP message from CloudEvent remains unchanged because the first slice of [][]byte is used to populate the data.

Ref: https://github.com/cloudevents/spec/issues/1275

embano1 commented 1 month ago

cc/ @deissnerk

embano1 commented 1 month ago

cc/ @duglin for review

duglin commented 1 month ago

The code change itself looks good w.r.t. the "just concat them" decision. However, @deissnerk, is there really no way for someone to split the result back into separate data sections? A simple concat w/o any kind of separator seems like it would make it really hard for consumers, no?

What's the point of using separate sections if once it becomes a CE it's just one long array of bytes?

deissnerk commented 1 month ago

@duglin I think the option to have multiple data sections is due to AMQP's feature of splitting a message into multiple transfer frames. I don't think it has relevance beyond this. @clemensv Is my understanding correct?

embano1 commented 1 month ago

@deissnerk that's how I understood it as well

duglin commented 1 month ago

ok in that case LGTM