-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
APIs changed to refer to subjects. Retrieval by globally unique schema id #30
Conversation
…dded the ability to assign globally unique schema id on the master and retrieve schemas using ids 3. Unit tests need to be modified
…ves to be too memory intensive during performance testing, we can change it to a memory mapped index constructed from scratch during bootstrap 2. All tests pass. Changed tests to validate schemas based on ids. Also included tests that get the same schema using id and subject,version
@junrao Best if you review this pull request |
{"schema":"Hello World"}' | ||
|
||
curl -v -H "Content-Type: application/vnd.schemaregistry.v1+json" -X POST -i http://localhost:8080/subjects/Kafka,key/versions -d ' | ||
{"schema":"Kafka"}' | ||
|
||
5. List all topics |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
topics should be subjects.
…nly unique identifier for a schema is the subject and the version
@junrao Addressed your review comments. I'm ready to merge unless you want to take another look. |
|
||
8. Get a particular schema using it's id (as returned by the register request) | ||
curl -v -H "Content-Type: application/vnd.schemaregistry.v1+json" -X GET http://localhost:8080/subjects/0 | ||
curl -v -H "Content-Type: application/vnd.schemaregistry.v1+json" -X GET http://localhost:8080/subjects/1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is item 8 still needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will remove during the push
Thanks for the review. Pushed to master |
Fixes #29