Support ForSchemaOption to call avro.Type.forSchema() #47
Conversation
|
@tulios @erikengervall any comment? |
src/SchemaRegistry.ts
Outdated
| @@ -28,9 +29,10 @@ export default class SchemaRegistry { | |||
| private api: SchemaRegistryAPIClient | |||
| public cache: Cache | |||
|
|
|||
| constructor({ auth, clientId, host, retry }: SchemaRegistryAPIClientArgs) { | |||
| constructor({ auth, clientId, host, retry }: SchemaRegistryAPIClientArgs, | |||
| forSchemaOptions?: Partial<ForSchemaOptions>) { | |||
There was a problem hiding this comment.
Could this second parameter be an object instead? That way it'd be more easily extendable if we wish to add more opts in the future :)
There was a problem hiding this comment.
could we just add to SchemaRegistryAPIClientArgs?
There was a problem hiding this comment.
I wouldn’t want to mix this into the APIClient options since it’s a completely separate thing.
That’s why I suggested adding a second argument, and to make it future-safe it’d be an object 🙂
There was a problem hiding this comment.
Agree, just pushed again :-)
Sorry for the delay I've reviewed the PR now 👍 |
|
@erikengervall @tulios please review again, thanks! |
src/SchemaRegistry.ts
Outdated
| @@ -28,9 +28,9 @@ export default class SchemaRegistry { | |||
| private api: SchemaRegistryAPIClient | |||
| public cache: Cache | |||
|
|
|||
| constructor({ auth, clientId, host, retry }: SchemaRegistryAPIClientArgs) { | |||
| constructor({ auth, clientId, host, retry, forSchemaOptions }: SchemaRegistryAPIClientArgs) { | |||
There was a problem hiding this comment.
As discussed previously: introduce a second argument instead as an object.
So that we can use @ovotech/avro-logical-types with @kafkajs/confluent-schema-registry.
|
could you bump the version in package.json? thanks! |
Absolutely - it's on its way 🙂 |
So that we can use @ovotech/avro-logical-types with @kafkajs/confluent-schema-registry.