bencebalogh / avro-schema-registry

Confluent Schema Registry implementation in javascript to easily serialize and deserialize kafka messages
MIT License
28 stars 30 forks source link

No way to pass "type" as argument to fetch latest schema by TopicName #36

Closed sorabhm closed 4 years ago

sorabhm commented 4 years ago

module.exports = (registry, topic, parseOptions = null, type = 'value') Hardcoding of "type" argument to "value" blocking from sending a custom type to fetch the schema. Is there a known way or fix underway?

bencebalogh commented 4 years ago

It's the default, but you can change it to key if you want. In the code the type will be appended to the end of the topic name here, do you mean you want to use a path that's like ${path}subjects/${topic}/versions (so no hyphen then anything)? If yes then I could fix it by if you set the type to '' it won't append the hyphen either to the topic name.

sorabhm commented 4 years ago

Thanks bence, I understand that it's the default which is always enforced. Because using the library, type EncodeMessage does not allow me to input the "type" or may be I am using it incorrectly. Definitely the way logic is written if type is empty it still appends "-" which can be avoided. Another request is if you can also expose "pushSchema" or registerSchema feature, it will be a good addon. I can see that functionality is already there, it is just not exposed.

bencebalogh commented 4 years ago

Aah, okay, it's been a while since I've seen this code, I see what you mean now, it's incorrect indeed, if you refer to (this)[https://github.com/bencebalogh/avro-schema-registry/blob/master/lib/get-latest-schema-version.js#L60]

Since this function is only used from getSchemaByTopicName it'd make sense if it's changed as:

getSchemaByTopicName should accept an optional new parameter type, that's default value is value, so we don't break the current behavior if someone updates to the new version. If that parameter is set then it'll append -${type} to the end of the topic name, if it's set to null or '' then it won't append anything.

I'll open a PR soon if noone else does.

I can include exposing the other 2 methods you mentioned as well in a separate PR.

sorabhm commented 4 years ago

Perfect thanks. I will see that if I could contribute here to the new features.

Given i am restricted with my client's compliance requirements so fetching the latest version might not be possible in the short period, hence I have implemented a custom method to create a schema.

bencebalogh commented 4 years ago

Should be fixed by #42 , currently making sure it won't break anything

bencebalogh commented 4 years ago

should be fixed in 2.1.0