[GitHub] mgodave commented on issue #1381: Schema registry 4/N

[GitBox <gi...@apache.org>]

mgodave commented on issue #1381: Schema registry 4/N
URL: https://github.com/apache/incubator-pulsar/pull/1381#issuecomment-373115018
 
 
   Hold on, I've made a lot of these changes but I think my last two branches
   have misplaced commits. I might just merge them into one last pr.
   
   On Wed, Mar 14, 2018 at 11:34 AM, Matteo Merli <no...@github.com>
   wrote:
   
   > *@merlimat* commented on this pull request.
   > ------------------------------
   >
   > In pulsar-broker/src/main/java/org/apache/pulsar/broker/
   > admin/SchemasResource.java
   > <https://github.com/apache/incubator-pulsar/pull/1381#discussion_r174545638>
   > :
   >
   > > +import javax.ws.rs.Path;
   > +import javax.ws.rs.PathParam;
   > +import javax.ws.rs.Produces;
   > +import javax.ws.rs.container.AsyncResponse;
   > +import javax.ws.rs.container.Suspended;
   > +import javax.ws.rs.core.MediaType;
   > +import javax.ws.rs.core.Response;
   > +import org.apache.pulsar.broker.service.Topic;
   > +import org.apache.pulsar.broker.web.RestException;
   > +import org.apache.pulsar.common.naming.TopicName;
   > +import org.apache.pulsar.common.schema.SchemaData;
   > +import org.apache.pulsar.common.schema.SchemaType;
   > +import org.apache.pulsar.common.schema.SchemaVersion;
   > +
   > +@Path("/schemas")
   > +public class SchemasResource extends AdminResource {
   >
   > This handler should go on the v2 admin API
   > ------------------------------
   >
   > In pulsar-broker/src/main/java/org/apache/pulsar/broker/
   > admin/SchemasResource.java
   > <https://github.com/apache/incubator-pulsar/pull/1381#discussion_r174546254>
   > :
   >
   > > +@Path("/schemas")
   > +public class SchemasResource extends AdminResource {
   > +
   > +    private final Clock clock;
   > +
   > +    @VisibleForTesting
   > +    SchemasResource(Clock clock) {
   > +        super();
   > +        this.clock = clock;
   > +    }
   > +
   > +    public SchemasResource() {
   > +        this(Clock.systemUTC());
   > +    }
   > +
   > +    @GET @Path("/{property}/{cluster}/{namespace}/{topic}/schema")
   >
   > For existing REST API handlers, we have split the endpoints in 2:
   >
   >    - One with the old topic name
   >    - One with v2 topic name (no cluster in the name).
   >
   > I think that for this new feature we could just concentrate on v2 API.
   > ------------------------------
   >
   > In pulsar-broker/src/main/java/org/apache/pulsar/broker/
   > admin/SchemasResource.java
   > <https://github.com/apache/incubator-pulsar/pull/1381#discussion_r174546539>
   > :
   >
   > > +                .isDeleted(false)
   > +                .timestamp(clock.millis())
   > +                .type(SchemaType.valueOf(payload.type))
   > +                .user(clientAppId())
   > +                .build()
   > +        ).thenAccept(version ->
   > +            response.resume(
   > +                Response.accepted().entity(
   > +                    new PostSchemaResponse(version)
   > +                ).build()
   > +            )
   > +        );
   > +    }
   > +
   > +    private String buildSchemaId(String property, String cluster, String namespace, String topic) {
   > +        return property + "_" + cluster + "_" + namespace + "_" + topic;
   >
   > I think we already added a method to get the schema in TopicName, right?
   > ------------------------------
   >
   > In pulsar-broker/src/main/java/org/apache/pulsar/broker/
   > admin/SchemasResource.java
   > <https://github.com/apache/incubator-pulsar/pull/1381#discussion_r174546852>
   > :
   >
   > > +            } else {
   > +                throw e;
   > +            }
   > +        }
   > +    }
   > +
   > +    private void validateDestinationExists(TopicName dn) {
   > +        try {
   > +            Topic topic = pulsar().getBrokerService().getTopicReference(dn.toString());
   > +            checkNotNull(topic);
   > +        } catch (Exception e) {
   > +            throw new RestException(Response.Status.NOT_FOUND, "Topic not found");
   > +        }
   > +    }
   > +
   > +    static class GetSchemaResponse {
   >
   > If this classes will have to be used in the PulsarAdmin Java client, they
   > should be added in pulsar-common module.
   >
   > —
   > You are receiving this because you authored the thread.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/incubator-pulsar/pull/1381#pullrequestreview-103922460>,
   > or mute the thread
   > <https://github.com/notifications/unsubscribe-auth/AAh9FtDtudZWOI4D9-5tpC5F8snJJps1ks5teVTBgaJpZM4SqqD8>
   > .
   >
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services