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

Feat: Download a specific version of schemas by maven plugin #1701

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

danielpetisme
Copy link

This PR aims at providing a way to download a specific version of a subject rather than the last one.

@ghost
Copy link

ghost commented Dec 8, 2020

It looks like @danielpetisme hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement here.

Once you've signed reply with [clabot:check] to prove it.

Appreciation of efforts,

clabot

@danielpetisme
Copy link
Author

Feel free to correct me, but from my understanding the current DownloadSchemaRegistryMojoTest does not test anything...
There is no mention of .execute to trigger the mojo behavior.
So the very first step was to baseline the current expected behavior with a running test.

@danielpetisme danielpetisme changed the title Fixing DownloadSchemaRegistryMojoTest tests Feat: Download a specific version of schemas by maven plugin Dec 9, 2020
@danielpetisme
Copy link
Author

I proposed a first very pragmatic approach.
As fore now, the subjectPatterns argument is mandatory for the MOJO config
https://github.com/confluentinc/schema-registry/blob/master/maven-plugin/src/main/java/io/confluent/kafka/schemaregistry/maven/DownloadSchemaRegistryMojo.java#L52

To prevent breaking changes I propose to define a subejct's version with a :<int> suffix on the pattern (it's an option I'm familiar with Gradle.
The following configurations are valid

<subjectPatterns>
  <param>^TestSubject000-(key|value)$</param>
  <param>^TestSubject001-(key|value)$:42</param>
</subjectPatterns>

if the separator : is not present, the MOJO will consider the user is expecting the latest version of the subject (just as it work right now). If : is present then the second part of the string will be considered as the a version (casted to integer).
When a version is defined, it will be associated to all the subjects matching the pattern (in the above example, Maven will download the version 42 for both TestSubject001-key and TestSubject001-value subjects.

The code can be improved but I really wanted to prototype a behavior to open the discussion.

@danielpetisme
Copy link
Author

As explained the main ambition of this prototype was to not introduce a breaking change. However I must admit the Developer experience is not good with this solution.
The best approach would be to proposes new properties to let the user be more explicit, like

<subjects>
  <subject>
   <!--- full explicit -->
    <name>TestSubject000-key</name>
    <version>42</version>
    <format>avro</format>
  </subject>
  <subject>
   <!--- version: latest, format: default/avro -->
    <name>TestSubject000-key</name>
  </subject>
  <subject>
    <!--- name and pattern would be mutually exclusive, format: default/avro -->
    <pattern>TestSubject100-(key|value)</pattern>
    <version>42</version>
  </subject>
  <subject>
    <!--- version: latest, format: default/avro -->
    <pattern>TestSubject100-(key|value)</pattern>
  </subject>
<subjects>

WDYT?

@cla-assistant
Copy link

cla-assistant bot commented Sep 25, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant