kafkajs / confluent-schema-registry

is a library that makes it easier to interact with the Confluent schema registry
https://www.npmjs.com/package/@kafkajs/confluent-schema-registry
MIT License
157 stars 102 forks source link

Register a schema unnecessary updates the compatibility #129

Open WtfJoke opened 3 years ago

WtfJoke commented 3 years ago

In our system were not allowed to update compatibility for a given subject. When we tried to register a new schema, we couldn't register it, because the library tried to update the compatibility (which resulted in a 403) and therefore never registered the new schema.

https://github.com/kafkajs/confluent-schema-registry/blob/master/src/SchemaRegistry.ts#L118-L133

Is there a way to bypass the update of a compatibility? Or wouldn't it be better to split the register calls and the update compatibility api calls in two different methods?

Nevon commented 3 years ago

A way to bypass this would be to register your schemas with the compatibility that is set on your subject.

const {
  COMPATIBILITY: { FORWARD },
} = require('@kafkajs/confluent-schema-registry')

await registry.register(schema, { compatibility: FORWARD })

Then the register method wouldn't detect a difference and won't try to update the subject.

Or wouldn't it be better to split the register calls and the update compatibility api calls in two different methods?

With the benefit of hindsight, we should have not had a default compatibility level. We chose BACKWARDS based on https://docs.confluent.io/platform/current/schema-registry/develop/api.html#compatibility, and now I'm afraid we're stuck with it unless we wanna make a breaking change.

Another option would be to allow compatibility: null or similar, so that if compatibility is undefined we fall back to the BACKWARDS default value, and if it's null we don't check/update the compatibility level of the subject.

WtfJoke commented 3 years ago

A way to bypass this would be to register your schemas with the compatibility that is set on your subject.

The subject doesnt exist yet, we tried to register it for the first time.

Another option would be to allow compatibility: null

We did that as a workaround.

We chose BACKWARDS based on https://docs.confluent.io/platform/current/schema-registry/develop/api.html#compatibility, and now I'm afraid we're stuck with it unless we wanna make a breaking change.

Im not sure if that applies to all schema registries, but when we register the subject manually, we dont need to set compatibility at all (its defaulted automatically to BACKWARDS)

AlexeyRaga commented 1 year ago

It looks to me that setting the compatibility level to null when a "server default" case is required isn't a problem.

The problem here is that the client currently does this:

if (compatibilityLevel.toUpperCase() !== compatibility) {
    throw new errors_1.ConfluentSchemaRegistryCompatibilityError(`Compatibility does not match the configuration (${compatibility} != ${compatibilityLevel.toUpperCase()})`);
}

Why are we doing it? Shouldn't we just rely on SchemaRegistry judgement on whether the schema is acceptable or not?

Or, at least, can we make this check conditional and, say, not perform the check and not throw if compatibility is set to null explicitly?

Would you be willing to accept such a PR?

//cc @Nevon