cloudevents / sdk-python

Python SDK for CloudEvents
https://pypi.org/p/cloudevents/
Apache License 2.0
266 stars 53 forks source link

`cloudevents.kafka.to_binary` checks for a `content-type` attribute, which would not be a valid attribute name #211

Closed aucampia closed 1 month ago

aucampia commented 1 year ago

https://github.com/cloudevents/sdk-python/blob/ef982743b68866abbe0049dbffac76f5a2e3efb4/cloudevents/kafka/conversion.py#L90

This will call CloudEvent._get_attributes()["content-type"] but a content-type attribute will violate the attribute naming rules:

https://github.com/cloudevents/spec/blob/30f1a081743bfe06b45b85519dd145f14a1ad22c/cloudevents/spec.md?plain=1#L173-L175

CloudEvents attribute names MUST consist of lower-case letters ('a' to 'z') or digits ('0' to '9') from the ASCII character set. Attribute names SHOULD be descriptive and terse and SHOULD NOT exceed 20 characters in length.

It is not really that clear to me why this is being done, but it also causes some problems when event is a Pydantic cloud event, but regardless of that it seems pretty clear that it is wrong.

xSAVIKx commented 1 year ago

Hey @aucampia, thx for raising this. I believe you're correct and the event itself must not contain the content-type attribute. The conversion here should convert the datacontenttype of the event to the content-type header of the Kafka message.

Ans similarly the backwards conversion must take care of the respective mapping.

Here's the supportive docs section: https://github.com/cloudevents/spec/blob/30f1a081743bfe06b45b85519dd145f14a1ad22c/cloudevents/bindings/kafka-protocol-binding.md#321-content-type

duglin commented 1 year ago

someone want to PR this?

aucampia commented 1 year ago

I want to, but realistically I probably won't get to it before the end of next week, though I may look at it then. Will notify before I start working on it though.

xSAVIKx commented 1 year ago

I'm out of capacity till the end of next week as well. Even though it's a small change in the codebase, I'd appreciate a PR. If none will start working on it, I'll find some time next week.

MaryamTaj commented 7 months ago

Hi! I would be happy to work on this!