flipp-oss / deimos

Framework to work with Kafka, Avro and ActiveRecord
Other
59 stars 22 forks source link

Use string-safe encoding for partition keys #96

Closed jmccance closed 3 years ago

jmccance commented 3 years ago

Pull Request Template

Description

When using the key as the partition key in the database producer backend, encode as ~JSON~ YAML to avoid ActiveRecord quoting issues.

Fixes #95

Type of change

Please delete options that are not relevant.

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Checklist:

jmccance commented 3 years ago

@dorner In #95 we talked about using to_yaml. I went with to_json here, thinking that it's (1) easier to read in a single line and (2) how Avro objects are typically presented for human consumption. If you'd prefer we stick to_yaml, let me know and I'll update.

dorner commented 3 years ago

Hey @jmccance! I'd actually prefer that we go back to to_yaml, because otherwise it's going to introduce a breaking change - e.g. if the partition key for ID 1 used to be a hash of "string A" and now it's a hash of "string B", that will evaluate to a different partition and it'll break the partitioning schema when we update.

Thanks a bunch! 😄

jmccance commented 3 years ago

@dorner Totally reasonable. Updated to use to_yaml.

dorner commented 3 years ago

LGTM!

dorner commented 3 years ago

Version 1.8.3 has been released - thanks so much for the contribution!