[GitHub] ivankelly commented on a change in pull request #1232: Schema registry (1/4)
ivankelly commented on a change in pull request #1232: Schema registry (1/4) URL: https://github.com/apache/incubator-pulsar/pull/1232#discussion_r168262139 ## File path: pulsar-common/src/main/proto/PulsarApi.proto ## @@ -368,6 +387,7 @@ message CommandRedeliverUnacknowledgedMessages { message CommandSuccess { required uint64 request_id = 1; + optional Schema schema = 2; Review comment: ok, in that case, this makes sense. perhaps it should be a repeated rather than an optional though, for the case that there's different versions. I guess that the schema name must be immutable for a topic then also. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ivankelly commented on a change in pull request #1232: Schema registry (1/4)
ivankelly commented on a change in pull request #1232: Schema registry (1/4) URL: https://github.com/apache/incubator-pulsar/pull/1232#discussion_r168240669 ## File path: pulsar-common/src/main/proto/PulsarApi.proto ## @@ -368,6 +387,7 @@ message CommandRedeliverUnacknowledgedMessages { message CommandSuccess { required uint64 request_id = 1; + optional Schema schema = 2; Review comment: Will the whole schema go with all messages? If that's the case, isn't this field here redundant? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ivankelly commented on a change in pull request #1232: Schema registry (1/4)
ivankelly commented on a change in pull request #1232: Schema registry (1/4) URL: https://github.com/apache/incubator-pulsar/pull/1232#discussion_r168196011 ## File path: pulsar-common/src/main/proto/PulsarApi.proto ## @@ -22,6 +22,20 @@ package pulsar.proto; option java_package = "org.apache.pulsar.common.api.proto"; option optimize_for = LITE_RUNTIME; +message Schema { + required string name = 1; Review comment: tabs 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ivankelly commented on a change in pull request #1232: Schema registry (1/4)
ivankelly commented on a change in pull request #1232: Schema registry (1/4) URL: https://github.com/apache/incubator-pulsar/pull/1232#discussion_r168230410 ## File path: pulsar-common/src/main/proto/PulsarApi.proto ## @@ -22,6 +22,20 @@ package pulsar.proto; option java_package = "org.apache.pulsar.common.api.proto"; option optimize_for = LITE_RUNTIME; +message Schema { + required string name = 1; + required bytes version = 2; + required bytes schema_data = 3; Review comment: Does the schema data itself have a schema? If we're supporting json, arvo, pb, etc, shouldn't we have a separate field to give the reader a hint of how they need to parse the schema? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ivankelly commented on a change in pull request #1232: Schema registry (1/4)
ivankelly commented on a change in pull request #1232: Schema registry (1/4) URL: https://github.com/apache/incubator-pulsar/pull/1232#discussion_r168229944 ## File path: pulsar-common/src/main/proto/PulsarApi.proto ## @@ -22,6 +22,20 @@ package pulsar.proto; option java_package = "org.apache.pulsar.common.api.proto"; option optimize_for = LITE_RUNTIME; +message Schema { + required string name = 1; + required bytes version = 2; + required bytes schema_data = 3; +repeated KeyValue properties = 4; +} + +message Tombstone {} + +message SchemaEntry { Review comment: I assume this is part of a the schema registry part? Why is tombstone separate from the schema? You have schema_data as required? If tombstoning a schema (which I assume is deleting the schema) what should the data be? Couldn't schema data itself being empty denote the tombstone? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ivankelly commented on a change in pull request #1232: Schema registry (1/4)
ivankelly commented on a change in pull request #1232: Schema registry (1/4) URL: https://github.com/apache/incubator-pulsar/pull/1232#discussion_r168231228 ## File path: pulsar-common/src/main/proto/PulsarApi.proto ## @@ -368,6 +387,7 @@ message CommandRedeliverUnacknowledgedMessages { message CommandSuccess { required uint64 request_id = 1; + optional Schema schema = 2; Review comment: what happens if the message schema changes mid topic? 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: us...@infra.apache.org With regards, Apache Git Services