Api update #70
Api update #70
Conversation
| } | ||
|
|
||
| @GET | ||
| @Path("/{version}") | ||
| public Schema getSchema(@PathParam("version") Integer version) { | ||
| @Path("/{id}") |
There was a problem hiding this comment.
I thought we decided this path should be /schemas/ids/?
There was a problem hiding this comment.
Good catch. Fixed!
…rsions/{version} resource that returns true if the input schema is compatible with the given version and false otherwise. Avro doesn't seem to have a way to expose the details about the incompatibility so we may not be able to return the detailed error message. 2. Added GET /schemas/{id} 3. Added POST /subjects/{subject} that returns the version of the schema if the input schema was already registered under the specified subject 4. Modified tests 5. Added the apache staging maven repository to get the 0.8.2.0 dependency to work. Once 0.8.2.0 is released, we can remove that
| return schema; | ||
| SubjectSchemaVersionResponse schemaVersionResponse = new SubjectSchemaVersionResponse(); | ||
| schemaVersionResponse.setVersion(version); | ||
| asyncResponse.resume(schemaVersionResponse); |
There was a problem hiding this comment.
What about returning a Schema object (which has string, id, version)? That way the response is a little more uniform/less seemingly arbitrary.
Query Result
subject + schemastring -> SchemaRecord (POST /subjects/ payload is schemastring)
(subject + id (optional) -> SchemaRecord (POST /subjects/ payload is id))
subject + version -> SchemaRecord (GET /subjects//versions/)
…chema is registered under different subjects
| @@ -125,7 +128,6 @@ | |||
| } | |||
|
|
|||
|
|
|||
|
|
|||
| private static int registerSchema(String baseUrl, Map<String, String> requestProperties, | |||
| RegisterSchemaRequest registerSchemaRequest, String subject, | |||
| boolean isDryRun) | |||
There was a problem hiding this comment.
Looks like a bunch of dryrun logic is still present - are you planning on getting rid of that in this branch?
There was a problem hiding this comment.
hmm, I already made those changes. Perhaps those are still local to my branch. Should be visible in the updated pull request
nehanarkhede
force-pushed
the
api-update
branch
from
3411c50
to
004df45
Compare
|
@granders Ready for a re-review. The open issues that I know of that I avoided fixing in this patch are -
|
|
Looks like you removed a bunch of dryrun logic but some remains (as far as I can tell I have the latest): Geoffreys-MacBook-Pro:schema-registry geoffreyanderson$ grep -ril dryrun . | sort -u | grep -v target | grep -v idea |
|
Sorry, found one last remnant of dry run! egrep -ril "dry[-_]?run" . | sort -u | grep -v target | grep -v idea Otherwise good to merge |
|
Thanks for the reviews, @granders |
Fixes #48.
@granders Ready for you to review. Looking for feedback on naming of the new classes, resource and the path parameters.