feat: first support of bindings for AVRO key #77
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
M3lkior
force-pushed
the
feat/67-first-support-of-kafka-key-bindings
branch
from
dcfcc72
to
721731f
Compare
M3lkior
changed the title
M3lkior
force-pushed
the
feat/67-first-support-of-kafka-key-bindings
branch
from
721731f
to
6cf8ca3
Compare
index.js
Outdated
| @@ -7,6 +7,19 @@ module.exports.parse = async ({ message, defaultSchemaFormat }) => { | |||
| message['x-parser-original-payload'] = message.payload; | |||
| message.payload = transformed; | |||
| delete message.schemaFormat; | |||
|
|
|||
| async function handleProtocolKey() { | |||
| for (const protocol of Object.keys(message.bindings)) { | |||
There was a problem hiding this comment.
I think we can go straight to Kafka protocol instead of iterating. I don't know any other protocol that has a key property that can be defined with Avro.
There was a problem hiding this comment.
if fact this was my first approach but i guess maybe new EDI technologies in future can handle message key like nats.io ... but ok i will change this implementation to only support the kafka protocol
|
Maybe as a temporary solution to this problem it is ok, but in the long term it is not a good solution. We can now hardcode with that, but what if someone defines a key in kafka with EDIT: Related issue with proposal asyncapi/spec#622 |
Yes i agree with you. i assume that the |
|
Looks good to me so far. I think we should update the Kafka binding before we proceed to merge this one: https://github.com/asyncapi/bindings/tree/master/kafka. |
By indicating that the key can also handle an Avro reference file in addition to a Schema Object ? |
|
Yes, that's what I was thinking. As the binding is right now, this library would not be compliant but since I think it's a great thing to have for Kafka devs, maybe it's just a matter of updating the Kafka binding definition. |
Here is the related PR @fmvilas asyncapi/bindings#81 |
|
Guys (@fmvilas / @magicmatatjahu ) , what about this PR status linked to the merge asyncapi/bindings#81 ? Could you merge it ? |
|
@fmvilas You requested changes one time :) Please look again. |
|
@M3lkior there are some linter errors. Otherwise, it looks good to me. |
oups sorry, it is fixed ! |
|
@magicmatatjahu can you review and approve/reject? |
Co-authored-by: Maciej Urbańczyk <urbanczyk.maciej.95@gmail.com>
M3lkior
dismissed stale reviews from magicmatatjahu and fmvilas
via
07716bf
|
Kudos, SonarCloud Quality Gate passed!
|
|
🎉 This PR is included in version 0.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
see the #67 for the details ;
this implementation only support
bindings.[protocol].keybut if you have any feedback for a better support of general bindings for any property ; i'm listeningHere is the result with the support of bindings for my kafka key definition that was previously not correctly displayed in the html generation (test with the last version of the generator)