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 2020/04/20 14:30:14 UTC
[GitHub] [pulsar] wolfstudy opened a new pull request #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
wolfstudy opened a new pull request #6776:
URL: https://github.com/apache/pulsar/pull/6776
Signed-off-by: xiaolong.ran <rx...@apache.org>
Fixes #6759
### Modifications
- Fix the Create subscribtion swagger of PersistentTopic
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wolfstudy commented on issue #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on issue #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-618151618
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] hangc0276 commented on pull request #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-618784699
> @hangc0276 Good catch! @wolfstudy could please help take a look the last comment? maybe you need to move the `messageId` to the end of params list.
Thanks for your feedback. @wolfstudy maybe you should implement a constructor for `MessageIdImpl`，which param is a string, and you should parse string to `MessageIdImpl` field, such as ledgerId, entryId and partitionIndex. Because the QueryParam just pass a string as param to messgeId, you should construct a MessageIdImpl object using the string param.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wolfstudy commented on pull request #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-619373170
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] jiazhai commented on a change in pull request #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
jiazhai commented on a change in pull request #6776:
URL: https://github.com/apache/pulsar/pull/6776#discussion_r422950767
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
##########
@@ -798,14 +798,17 @@ public void createSubscription(
@PathParam("topic") @Encoded String topic,
@ApiParam(value = "Subscription to create position on", required = true)
@PathParam("subscriptionName") String encodedSubName,
- @ApiParam(value = "messageId where to create the subscription. " +
+ @ApiParam(value = "Is authentication required to perform this operation")
Review comment:
This is wrong. You are changing the parameter order.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-625309847
@wolfstudy Could you please take a look at the CI?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wolfstudy commented on issue #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on issue #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-616941725
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wolfstudy commented on pull request #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-619388651
> But i am afraid your present implementation will compile failed due to lack of `MessageIdImpl` constructor for `@QueryParam("messageId") @DefaultValue("latest") MessageIdImpl messageId,`
Yes i was wrong, maybe we should proceed in the following way:
```
@ApiParam(name = "messageId", value = "messageId where to create the subscription. " +
"It can be 'latest', 'earliest' or (ledgerId:entryId)",
defaultValue = "latest",
allowableValues = "latest,earliest,ledgerId:entryId"
)
MessageIdImpl messageId,
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wolfstudy commented on issue #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on issue #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-617650131
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-618764842
@hangc0276 Good catch! @wolfstudy could please help take a look the last comment? maybe you need to move the `messageId` to the end of params list.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wolfstudy commented on pull request #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-619373142
> > @hangc0276 Good catch! @wolfstudy could please help take a look the last comment? maybe you need to move the `messageId` to the end of params list.
>
> Thanks for your feedback. @wolfstudy maybe you should implement a constructor for `MessageIdImpl`，which param is a string, and you should parse string to `MessageIdImpl` field, such as ledgerId, entryId and partitionIndex. Because the QueryParam just pass a string as param to messgeId, you should construct a MessageIdImpl object using the string param.
Good idea, maybe we can send a new issue to track this, the proposal will change the current default behavior. We can modify it in a separate pull request.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wolfstudy commented on pull request #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-626128514
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] jiazhai commented on pull request #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-626527768
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] hangc0276 commented on pull request #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-619380515
> > > @hangc0276 Good catch! @wolfstudy could please help take a look the last comment? maybe you need to move the `messageId` to the end of params list.
> >
> >
> > Thanks for your feedback. @wolfstudy maybe you should implement a constructor for `MessageIdImpl`，which param is a string, and you should parse string to `MessageIdImpl` field, such as ledgerId, entryId and partitionIndex. Because the QueryParam just pass a string as param to messgeId, you should construct a MessageIdImpl object using the string param.
>
> Good idea, maybe we can send a new issue to track this, the proposal will change the current default behavior. We can modify it in a separate pull request.
But i am afraid your present implementation will compile failed due to lack of `MessageIdImpl` constructor for `@QueryParam("messageId") @DefaultValue("latest") MessageIdImpl messageId,`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wolfstudy commented on pull request #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-620691797
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] hangc0276 commented on pull request #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-619477045
> > But i am afraid your present implementation will compile failed due to lack of `MessageIdImpl` constructor for `@QueryParam("messageId") @DefaultValue("latest") MessageIdImpl messageId,`
>
> Yes i was wrong, maybe we should proceed in the following way:
>
> ```
> @ApiParam(name = "messageId", value = "messageId where to create the subscription. " +
> "It can be 'latest', 'earliest' or (ledgerId:entryId)",
> defaultValue = "latest",
> allowableValues = "latest,earliest,ledgerId:entryId"
> )
> MessageIdImpl messageId,
> ```
This implementation will not expose messageId to Query param, maybe we'd better to use another issue to track it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-625279698
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wolfstudy commented on pull request #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-618772842
Thanks @hangc0276 feedback, fixed done, 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wolfstudy removed a comment on pull request #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
wolfstudy removed a comment on pull request #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-618772842
Thanks @hangc0276 feedback, fixed done, 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wolfstudy commented on pull request #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-618775410
@hangc0276 @codelipenghui Thanks feedback, fixed done, 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wolfstudy commented on pull request #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-626497881
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on issue #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
sijie commented on issue #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-617373473
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] hangc0276 commented on issue #6776: [Docs] Fix the Create subscribtion swagger of PersistentTopic
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on issue #6776:
URL: https://github.com/apache/pulsar/pull/6776#issuecomment-618164214
Whether `messageId` should be expose to Query Parameters? In your implementation, the `messageId` parameter doesn't expose to user?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org