-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Schema Registry Avro] Providing a message encoder #18365
[Schema Registry Avro] Providing a message encoder #18365
Conversation
e8fec65
to
1b10967
Compare
API changes have been detected in |
API changes have been detected in |
const decodedEvent = await encoder.decodeMessageData({ | ||
contentType: event.contentType, | ||
data: Uint8Array.from(Object.values(event.body)) | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To streamline the experience of decoding received messages, I am thinking of adding native Event Hubs received message type as a supported input type in decodeMessageData
. This could also apply to received message types in other messaging clients.
in JS, we can do that without importing by just duplicating the relevant bits of the native type. For instance, we can define the following input type for Event Hubs messages:
interface ReceviedEventHubsMessage {
body: any,
contentType?: string
}
/cc @JoshLove-msft
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about adding some kind of utility method to convert from the different native types to MessageWithMetadata?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But actually I like your idea better since it would be less work for the user.
// @public | ||
export class SchemaRegistryAvroEncoder { | ||
constructor(client: SchemaRegistry, options?: SchemaRegistryAvroEncoderOptions); | ||
decodeMessageData(message: MessageWithMetadata, readerSchema?: string): Promise<unknown>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably can just use "schema" as the param name.
Co-authored-by: JoshLove-msft <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Objects feels so much better in JS than opaque streams
### Breaking Changes | ||
- The `SchemaRegistryAvroSerializer` class has been renamed to `SchemaRegistryAvroEncoder` | ||
- The `serialize` method has been renamed to `encodeMessageData` and it now returns a `MessageWithMetadata` | ||
- The `deserialize` method has been renamed to `encodeMessageData` and it now takes a `MessageWithMetadata` as input |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo
- The `deserialize` method has been renamed to `encodeMessageData` and it now takes a `MessageWithMetadata` as input | |
- The `deserialize` method has been renamed to `decodeMessageData` and it now takes a `MessageWithMetadata` as input |
@@ -4,19 +4,23 @@ | |||
|
|||
```ts | |||
|
|||
/// <reference types="node" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
Fixes #20061 ## Overview Revamps the schema registry encoder to work on messages instead of buffers based on the recommendation of the Azure messaging architect. This changes the APIs as follows: ```ts const buffer: NodeJS.Buffer = await serializer.serialize(value, schema); ``` becomes ```ts const message: MessageWithMetadata = await encoder.encodeMessageData(value, schema); ``` where `MessageWithMetadata` has a `body` field as well as a `contentType` field. The latter's format is `avro/binary+<Schema ID>` For derserializing, the change is as follows: ```ts const deserializedValue = await serializer.deserialize(buffer); ``` becomes: ```ts const decodedObject = await encoder.decodeMessageData(message); ``` ## Improvement upon #15959 This design introduces a new `messageAdapter` option in the encoder constructor to support processing of any message type (e.g. [cloud event](https://github.com/cloudevents/spec/blob/v1.0.1/spec.md)): ```ts const encoder = new SchemaRegistryAvroEncoder(schemaRegistryClient, { groupName, messageAdapter: adapter }); ``` where `adapter` is a message adapter that follows the following contract: ```ts interface MessageAdapter<MessageT> { produceMessage: (messageWithMetadata: MessageWithMetadata) => MessageT; consumeMessage: (message: MessageT) => MessageWithMetadata; } interface MessageWithMetadata { body: Uint8Array; contentType: string; } ``` For convenience, the PR adds a couple of convenience adapter factories for Event Hubs's `EventData` and Event Grid's `SendCloudEventInput<Uint8Array>`. For example, the `createCloudEventAdapter` factory can be called to construct an adapter for the latter as follows: ```ts const adapter = createCloudEventAdapter({ type: "azure.sdk.eventgrid.samples.cloudevent", source: "/azure/sdk/schemaregistry/samples/withEventGrid", }), ``` Note that these adapter factories are exported by their respective messaging package without explicitly implementing the contract and the PR adds new encoder tests that check whether the produced adapters follow the contract. This organization could change in the future if we create a new core place for the contract to be imported from. See the newly added samples for how to send such messages with Event Hubs and Event Grid. Schema Registry commitment tracking: #15959 Tracking issue: #18608 First iteration design: #18365
Closing in favor of #19842 |
Revamps the schema registry encoder to work on messages instead of buffers based on the recommendation of the Azure messaging architect.
This changes the APIs as follows:
becomes
where
MessageWithMetadata
has adata
field as well as acontentType
field. The latter's format isavro/binary+<Schema ID>
For derserializing, the change is as follows:
becomes:
See the newly added samples for how to send such messages with Event Hubs. See #19458 for updating Event Hubs to support such messages.
Note that #19458 needs to be merged first to enable the samples in this PR to typecheck.