You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/05/24 02:41:29 UTC
[GitHub] [pulsar] Demogorgon314 opened a new pull request, #15733: [fix][admin] Unwrap the completion exception on schemas admin API
Demogorgon314 opened a new pull request, #15733:
URL: https://github.com/apache/pulsar/pull/15733
### Motivation
Currently, the schemas admin API Implementation used raw exception to check the exception type, we should unwrap the completion exception.
### Modifications
Unwrap the completion exception on schemas admin API
### Documentation
Check the box below or label this PR directly.
Need to update docs?
- [ ] `doc-required`
(Your PR needs to update docs and you will update later)
- [x] `no-need-doc`
(Please explain why)
- [ ] `doc`
(Your PR contains doc changes)
- [ ] `doc-added`
(Docs have been already added)
--
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-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Demogorgon314 commented on pull request #15733: [fix][admin] Unwrap the completion exception on schemas admin API
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on PR #15733:
URL: https://github.com/apache/pulsar/pull/15733#issuecomment-1136612803
/pulsarbot run-failure-checks
--
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-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] BewareMyPower commented on a diff in pull request #15733: [fix][admin] Unwrap the completion exception on schemas admin API
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on code in PR #15733:
URL: https://github.com/apache/pulsar/pull/15733#discussion_r883708088
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiSchemaTest.java:
##########
@@ -247,6 +249,25 @@ public void createKeyValueSchema(ApiVersion version) throws Exception {
assertEquals(keyValueSchema.getSchemaInfo(), schemaInfo);
}
+
+ @Test(dataProvider = "version")
+ public void testInvalidSchemaDataException(ApiVersion version) {
+ String namespace = format("%s%s%s", "schematest", (ApiVersion.V1.equals(version) ? "/" + cluster + "/" : "/"),
+ "test");
+ String topicName = "persistent://"+ namespace + "/test-invalid-schema-data-exception";
+ SchemaInfo schemaInfo = SchemaInfo.builder()
+ .schema(new byte[0])
+ .type(SchemaType.AVRO)
+ .name("test")
+ .build();
+ try {
+ admin.schemas().createSchema(topicName, schemaInfo);
Review Comment:
Actually we should add the `fail()` call 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-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Demogorgon314 commented on a diff in pull request #15733: [fix][admin] Unwrap the completion exception on schemas admin API
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on code in PR #15733:
URL: https://github.com/apache/pulsar/pull/15733#discussion_r884051017
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiSchemaTest.java:
##########
@@ -247,6 +249,25 @@ public void createKeyValueSchema(ApiVersion version) throws Exception {
assertEquals(keyValueSchema.getSchemaInfo(), schemaInfo);
}
+
+ @Test(dataProvider = "version")
+ public void testInvalidSchemaDataException(ApiVersion version) {
+ String namespace = format("%s%s%s", "schematest", (ApiVersion.V1.equals(version) ? "/" + cluster + "/" : "/"),
+ "test");
+ String topicName = "persistent://"+ namespace + "/test-invalid-schema-data-exception";
+ SchemaInfo schemaInfo = SchemaInfo.builder()
+ .schema(new byte[0])
+ .type(SchemaType.AVRO)
+ .name("test")
+ .build();
+ try {
+ admin.schemas().createSchema(topicName, schemaInfo);
Review Comment:
Oh, the PR gets merged. I can fix it in other 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-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- merged pull request #15733: [fix][admin] Unwrap the completion exception on schemas admin API
Posted by GitBox <gi...@apache.org>.
Technoboy- merged PR #15733:
URL: https://github.com/apache/pulsar/pull/15733
--
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-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Demogorgon314 commented on pull request #15733: [fix][admin] Unwrap the completion exception on schemas admin API
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on PR #15733:
URL: https://github.com/apache/pulsar/pull/15733#issuecomment-1139473050
@BewareMyPower @gaoran10 I've added unit test, 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-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Demogorgon314 commented on pull request #15733: [fix][admin] Unwrap the completion exception on schemas admin API
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on PR #15733:
URL: https://github.com/apache/pulsar/pull/15733#issuecomment-1139472928
@BewareMyPower @gaoran10 I've added unit test, 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-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org