Skip to content
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

Optionaly return schema id together with decoded message #3

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

michaelr524
Copy link

Useful for changing the record and encoding back using the same schema.

@ngoyal
Copy link
Collaborator

ngoyal commented Feb 29, 2016

LGTM. Deferring to @stevemil00 or @sean to accept.

@stevemil00
Copy link

@michaelr524 could you give a bit more detailed example of the use case you're describing? I'm having trouble thinking offhand of a case where that'd be all that useful, and I can't tell whether or not that just means I need more caffeine... :)

@michaelr524
Copy link
Author

@stevemil00 we have a spark-streaming job consuming a kafka topi, transforming the messages and writes them back to another kafka topic.
messages are encoded in avro of course.
since python-confluent-schemaregistry decodes avro messages to plain dictionaries there is no way to figure out the schema these messages were encoded in the source topic and hence no easy way to encode them back using the same schema.
by having the schema id returned from the decoder, i call this to encode the message back, along the way reusing the schema from the registry client cache:
encoded_message = serializer.encode_record_with_schema_id(schema_id, record)

@stevemil00
Copy link

LGTM, sorry for losing track of this. I don't actually have permissions to merge here so if @ngoyal or @sean could either give me permissions or push the button, that'd be great. :)

@miguno
Copy link

miguno commented Mar 23, 2016

@stevemil00 : I don't have admin permissions anymore either (I left the GH org) so I can't help you out in the meantime.

@miguno
Copy link

miguno commented Mar 23, 2016

That said, you might want to ensure there are updated unit tests for this new functionality (see e.g. https://github.com/verisign/python-confluent-schemaregistry/blob/master/test/test_message_serializer.py). I'd say this is important because the return type of decode_message changes (!) depending on its input parameters -- from a single value to a tuple. (Personally, I would have perhaps preferred a separate method like decode_message_with_schema rather than using an optional parameter that changes a function's return type.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants