feat: embed avro schema id #3118
Conversation
fracasula
requested review from
ItsSudip,
atzoum,
cisse21,
lvrach,
Sidddddarth and
saurav-malani
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #3118 +/- ##
==========================================
+ Coverage 52.25% 52.30% +0.04%
==========================================
Files 318 318
Lines 52312 52390 +78
==========================================
+ Hits 27337 27400 +63
- Misses 23362 23368 +6
- Partials 1613 1622 +9
... and 7 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
| @@ -329,7 +332,15 @@ func NewProducer(destination *backendconfig.DestinationT, o common.Opts) (*Produ | |||
| if err != nil { | |||
| return nil, err | |||
| } | |||
| return &ProducerManager{p: p, timeout: o.Timeout, codecs: codecs}, nil | |||
|
|
|||
| // @TODO embedAvroSchemaID should come from control plane (i.e. destination config) | |||
There was a problem hiding this comment.
Shouldn't we add config-based support for this already & let it be overridden through this environment variable for the time being? That way, as soon as control plane is ready with their changes, the only thing we'll need to do is to remove the override.
There was a problem hiding this comment.
The configuration needs to be reworked anyway and if I understand correctly the whole thing will be picked up by @saurav-malani. He'll make it proper don't worry 👍
Description
Pull request to enable customers (i.e. CARS) to optionally have the schemaID embedded in the message payload.
This is compliant with the way Confluent libraries do it but it would break the workflow of existing customers using the Avro functionality (three at the moment). That is the reason why I decided to have it behind an optional flag (i.e.
embedAvroSchemaID bool).I've added a new
TestAvroSchemaRegistrywhere I'm testing both ways of sending Avro messages. One with the schemaID not embedded (existing customers) and one with the schemaID embedded (for CARS).Sidenote
This is now live in the customer namespace. It's been tested by them and they are happy with the changes.
Notion Ticket
< Notion Link >
Security