-
-
Notifications
You must be signed in to change notification settings - Fork 29
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
addReference() doesn't fully replace previous dependencies feature #57
Comments
Hello @denisw, Thank you for your interest in the plugin. I think I will also need to check if it's still possible on the schema registry side as It changed with the arrival of JSON and ProtoBuf support (which changed the API). |
@denisw I've just checked a bit more the code. The SchemaReference looks like this: public class SchemaReference implements Comparable<SchemaReference> {
private String name;
private String subject;
private Integer version;
...
} As you can see we cannot pass the schema directly in it. I've also checked on the Maven plugin, it does not support this feature either. In the meantime, would it be OK for your use case if the |
@ImFlog Sorry for the late reply, and thanks for looking into this more! My use case would not require any schema references on the remote side. I was really just looking for the previously supported feature where dependencies between Avro schema files were resolved locally and inlined before sending to Schema Registry - that is, the functionality introduced in #13. Maybe that is still supported and the arguments just passed a bit? Otherwise, I'm happy to look into re-implementing this at least for Avro. |
I think it's possible to add references locally and then send the request with the schema containing all it's dependencies in one go. A mix of local and remote could work if we have internal and external dependencies. Now that it's clearer for me, I will be able to start creating the structure for this mechanism. When done I will publish it like this if anyone volunteer for one of the implementation it will be easier :) I'll keep this ticket for this the structure part and then create 3 new ones for the respective implementation. |
Sounds good! Regarding the need for the serialization libraries of each data format, I can see three options:
|
This section of the documentation seems to fit what we need : https://docs.gradle.org/current/userguide/implementing_gradle_plugins.html#providing_default_dependencies_for_plugins I'm going on vacations for 3 weeks starting tomorrow and unfortunately I was not able to find time to work on this before. |
Sorry for the long delay. |
Any update on this? We are using the plugin in the same way, we do not need internal schema references to be registered remote. |
Hello @dejank1986, I just had a child and thus I didn't had much time to work on this. As this is not a quick fix I need to try to find more time but I did not start the design / implementation yet. |
Good new @denisw and @dejank1986, I took some time to look at this. |
@ImFlog That’s great to hear! Thank you so much for working on this. :) I am happy to test the PR in one of our production codebases, though I must admit that I never have installed a Gradle plugin from source, so I’d need to figure this out first. |
@ImFlog Great! Looking forward to new plugin release! :) |
Bad news, It seems that my first hack attempt didn't work because the register method only took the first schema and didn't take the local reference into account. I am currently trying with AVRO (with hugly hacks), but it's a bit of a rabbit hole. I have already implemented a custom AvroSchemaProvider that resolve the localReferences to String and made sure that the Avro parser read everything well but at one place of the code https://github.com/confluentinc/schema-registry/blob/master/client/src/main/java/io/confluent/kafka/schemaregistry/client/CachedSchemaRegistryClient.java#L491 Other lead would be the first thing I had in mind with using the Parser directly (might be the easiest). More information on the next episode |
I may have found something (again) but that would mean that if a user wants to use local + remote dependencies, It would be treated as a one big schema without remote dependencies. |
Good news @denisw @dejank1986, I don't know if you saw the PR changes but I have something working for AVRO (still need to check for Protobuf and Avro). |
@ImFlog Amazing! 🎉 Thank you so much for your continued work on this, despite all of the hurdles. It’s very appreciated. 😃 I’ll give it a spin in our projects as soon as the snapshot version is available. 👍 |
Okay one last issue, I was polishing the PR and I even released a version to give it a spin on the example folder. The only thing I can think of is to fetch the remote references and inline the whole schema before the call. |
@ImFlog I think this is a perfectly fine limitation. I don't expect us to mix local and remote references; in fact, it might be a good idea to explicitly not support this mixing for the same subject to avoid surprises. |
It's finally here @denisw ! The version 1.6.0 include the localReference support for AVRO. It took 2 month to fix, sorry for the delay. I will try to implement the other types faster :) Do not hesitate to reopen an issue if something is not working right. |
@ImFlog Great! 🎉🎉🎉 Thank you for pushing this! I will try it out in next days for sure. |
Before version 1.0, it was possible to split an Avro schema into multiple files and pass all of them to a single
subject()
call in theregister
block.The
addReference()
DSL method is meant to replace this, but it only works if the external reference is registered as a standalone subject in the Schema Registry. This is not the case in many of our uses, where we simply move the definition of a nested record type into a separate Avro file for convenience. This is not possible with the new DSL.It would be great if the previous set-of-Avro-files approach would be re-added to the DSL somehow, perhaps through an
addInternalReference()
method.The text was updated successfully, but these errors were encountered: