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

Moved key serialization to a JSON serializer. #33

Closed
wants to merge 5 commits into from
Closed

Conversation

nehanarkhede
Copy link
Contributor

Added support for config as well as schema keys. Added unit tests for testing ordering and key serialization. Fixes #15

…g as well as schema keys. Added uinit tests for testing ordering and key serialization
@nehanarkhede
Copy link
Contributor Author

@junrao Ready for you to review. Take a look at unit tests to understand the ordering between top-level config key, subject-level config key and schema key

@@ -0,0 +1,21 @@
/*
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please ignore this file. I will delete it during the merge.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

return compare;
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to be able to compare all combinations of different types of SchemaRegistryKey. Instead of duplicating the code, perhaps we can write all comparators in a dispatcher in SchemaRegistryKey once and just call the dispatcher in the compareTo() implementation of each type.

Perhaps we need to do the same for equals().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems counter intuitive. Both classes extend from SchemaRegistryKey. If 2 keys of different sub classes are being compared, they will be sorted based on keyType anyway. There is no duplication. Each sub class only compares according to it's own class members.

Same for equals

@nehanarkhede
Copy link
Contributor Author

@junrao Addressed all your review comments. I'd like to merge this to allow sending a pull request for making PUT/GET /config work. I will go ahead and do that if you don't have further review comments

@nehanarkhede
Copy link
Contributor Author

@junrao As discussed offline, I will include the json property ordering and the serialization into a map change in my new pull request.

@granders granders deleted the issue-15 branch March 6, 2015 21:25
rayokota added a commit that referenced this pull request Aug 20, 2024
* DGS-16357 Include default ctx in mock getContexts()

* Use distinct

* Fix checkstyle
rayokota added a commit that referenced this pull request Aug 20, 2024
DGS-16357 Include default ctx in mock getContexts() (#33)
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.

Use a key serializer in KafkaSchemaRegistry
2 participants