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

CCompat Layer the Schema dto is not fully compatible. #1660

Closed
michaelpearce-gain opened this issue Jul 12, 2021 · 11 comments · Fixed by #2272
Closed

CCompat Layer the Schema dto is not fully compatible. #1660

michaelpearce-gain opened this issue Jul 12, 2021 · 11 comments · Fixed by #2272
Assignees
Labels
type/bug Something isn't working

Comments

@michaelpearce-gain
Copy link

Missing two fields in the dto.

private String schemaType;
private List references;

See:
https://github.com/confluentinc/schema-registry/blob/master/client/src/main/java/io/confluent/kafka/schemaregistry/client/rest/entities/Schema.java

and:
https://github.com/Apicurio/apicurio-registry/blob/master/app/src/main/java/io/apicurio/registry/ccompat/dto/Schema.java

Note this means when using apicurio and the confluent clients, if using jsonschema or protobuf, its broken as it requires the schemaType, blocking migration to apicurio.

@michaelpearce-gain
Copy link
Author

michaelpearce-gain commented Jul 12, 2021

apicurio-confluent.zip

Please find attached a very simple test project that demonstrates issue with the missing two fields in ccompact using confluent serdes with protobuf.

@michaelpearce-gain
Copy link
Author

michaelpearce-gain commented Jul 12, 2021

@michaelpearce-gain
Copy link
Author

michaelpearce-gain commented Jul 12, 2021

seems you can see most of the changes that were added in 2020 here confluentinc/schema-registry#1280 which apicurio isnt current compatible with and yet the confluent serdes for json and proto rely on.

@carlesarnal
Copy link
Member

Thanks @michaelpearce-gain, I will look into that information, very helpful. As you said, it seems that we have a difference that is not documented in the API reference. For instance, if you go here or here you will see that the example response is not returning any schemaType or references.

I will revisit this and fix any issues we might have.

@carlesarnal
Copy link
Member

I have created two new issues to split the work into two separate pull requests. #1676 and #1675. Closing this one in favour of the other two.

@michaelpearce-gain
Copy link
Author

Can we keep this open so we get notified once resolved. Also it be good to include the sample test case we supplied in this issue to confirm resolvement.

@carlesarnal
Copy link
Member

Hi @michaelpearce-gain, the first issue is resolved, to return the schemaType. The second one, as you can imagine, is a tougher nut to crack since we don't have support for references yet in Apicurio Registry. That said, there are some discussions around implementing it.

See this.

@michaelandrepearce
Copy link

That makes it impossible to adopt / transition to it without. As for protobuf and jsonschema its a real prerequisite, thus why exists in confluents

@michaelandrepearce
Copy link

Hi @carlesarnal is there any indication if/when this will be addressed, as its quite a major blocker, and there is clearly prior art to take inspiration from, as the confluent one is working fine and agnostically for json and proto here.

@carlesarnal
Copy link
Member

Hi @michaelandrepearce, this is for sure in our backlog but I don't have a timeframe for the implementation.

@carlesarnal carlesarnal linked a pull request Feb 22, 2022 that will close this issue
@carlesarnal
Copy link
Member

Hi @michaelandrepearce, just FYI, the linked PR contributes the support for artifact references. In that PR, we added support for references in both the confluent compatibility API and also in the Apicurio Registry API and also in our Serdes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants