[GitHub] [pulsar-client-cpp] BewareMyPower commented on a diff in pull request #257: Support get the SchemaInfo from a topic and the schema version
BewareMyPower commented on code in PR #257:
URL: https://github.com/apache/pulsar-client-cpp/pull/257#discussion_r1179067778
##
include/pulsar/Message.h:
##
@@ -176,8 +176,16 @@ class PULSAR_PUBLIC Message {
*/
bool hasSchemaVersion() const;
+/**
+ * Get the schema version.
+ *
+ * @return the the schema version on success or -1 if the message does not
have the schema version
+ */
+int64_t getLongSchemaVersion() const;
+
/**
* Get the schema version
+ * @deprecated Use getLongSchemaVersion instead
Review Comment:
@RobertIndie PTAL again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar-client-cpp] BewareMyPower commented on a diff in pull request #257: Support get the SchemaInfo from a topic and the schema version
BewareMyPower commented on code in PR #257:
URL: https://github.com/apache/pulsar-client-cpp/pull/257#discussion_r1178552636
##
include/pulsar/Client.h:
##
@@ -404,6 +404,19 @@ class PULSAR_PUBLIC Client {
*/
uint64_t getNumberOfConsumers();
+/**
+ * Asynchronously get the SchemaInfo of a topic and a specific version.
+ *
+ * @topic the topic name
+ * @version the schema version byte array, see
Message::getLongSchemaVersion.
+ * @callback the callback that is triggered when the SchemaInfo is
retrieved successfully or not.
+ *
+ * NOTE: If there is no schema registered or the given version does not
exist, a SchemaInfo whose type is
Review Comment:
I won't add a new `Result`. The server side returns a `TopicNotFound` error.
https://github.com/apache/pulsar/blob/f7c0b3c49c9ad8c28d0b00aa30d727850eb8bc04/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientCnx.java#L999
So I use `ResultTopicNotFound` here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar-client-cpp] BewareMyPower commented on a diff in pull request #257: Support get the SchemaInfo from a topic and the schema version
BewareMyPower commented on code in PR #257:
URL: https://github.com/apache/pulsar-client-cpp/pull/257#discussion_r1178548743
##
include/pulsar/Client.h:
##
@@ -404,6 +404,19 @@ class PULSAR_PUBLIC Client {
*/
uint64_t getNumberOfConsumers();
+/**
+ * Asynchronously get the SchemaInfo of a topic and a specific version.
+ *
+ * @topic the topic name
+ * @version the schema version byte array, see
Message::getLongSchemaVersion.
+ * @callback the callback that is triggered when the SchemaInfo is
retrieved successfully or not.
+ *
+ * NOTE: If there is no schema registered or the given version does not
exist, a SchemaInfo whose type is
Review Comment:
In Java client, the `TopicNotFound` error is ignored and an empty optional
of `SchemaInfo` is returned. So I thought it should not be treated as an
exceptional case and users should process this error. I just use a `NONE`
schema to replace an empty optional.
But I just thought again and IMO an error should be returned if we cannot
get the schema for the given topic and version. So I will adopt the suggestion
you made so that users should not handle the schema type.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar-client-cpp] BewareMyPower commented on a diff in pull request #257: Support get the SchemaInfo from a topic and the schema version
BewareMyPower commented on code in PR #257:
URL: https://github.com/apache/pulsar-client-cpp/pull/257#discussion_r1178544663
##
include/pulsar/Message.h:
##
@@ -177,7 +177,14 @@ class PULSAR_PUBLIC Message {
bool hasSchemaVersion() const;
/**
- * Get the schema version
+ * Get the schema version.
+ *
+ * @return the the schema version on success or -1 if the message does not
have the schema version
+ */
+int64_t getLongSchemaVersion() const;
Review Comment:
Yes. The Java API design is terrible. It really harms the user experience.
Actually the byte array always represents a big-endian encoded 8 bytes integer,
unless it's empty, which indicates no schema version. I believe it's a mistake
made by the author. You can also see `Schemas#getSchemaInfo` in the
`pulsar-admin` module, you can see the parameter type is long.
```java
SchemaInfo getSchemaInfo(String topic, long version) throws
PulsarAdminException;
CompletableFuture getSchemaInfoAsync(String topic, long
version);
```
However, there is no document that tell users how to get the human-readable
representation of the byte array. Returning a byte array is definitely a
mistake. The Python client also wraps this API and returns a string to users.
And the users can nearly do nothing with it.
I will open a discussion later in the mail list. But for now, I think it's
good to add such a method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar-client-cpp] BewareMyPower commented on a diff in pull request #257: Support get the SchemaInfo from a topic and the schema version
BewareMyPower commented on code in PR #257:
URL: https://github.com/apache/pulsar-client-cpp/pull/257#discussion_r1177651271
##
include/pulsar/Message.h:
##
@@ -176,8 +176,16 @@ class PULSAR_PUBLIC Message {
*/
bool hasSchemaVersion() const;
+/**
+ * Get the schema version.
+ *
+ * @return the the schema version on success or -1 if the message does not
have the schema version
+ */
+int64_t getLongSchemaVersion() const;
+
/**
* Get the schema version
+ * @deprecated Use getLongSchemaVersion instead
Review Comment:
Okay, let's not mark it as deprecated in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
