[GitHub] [pulsar] BewareMyPower commented on a change in pull request #8724: [WebSocket] Fix the initial sequence id error

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

BewareMyPower commented on a change in pull request #8724:
URL: https://github.com/apache/pulsar/pull/8724#discussion_r531349653



##########
File path: pulsar-websocket/src/main/java/org/apache/pulsar/websocket/ProducerHandler.java
##########
@@ -298,7 +298,7 @@ private void updateSentMsgStats(long msgSize, long latencyUsec) {
         }
 
         if (queryParams.containsKey("initialSequenceId")) {
-            builder.initialSequenceId(Long.parseLong("initialSequenceId"));
+            builder.initialSequenceId(Long.parseLong(queryParams.get("initialSequenceId")));

Review comment:
       The `getProducerBuilder` cannot be tested simply. The parameter validation happens in `ProducerBuilder#create`. However the `PulsarClient`'s service url cannot be mocked because it retrieves the url related params from ZK, see `WebSocketService#getPulsarClient` for details.
   
   Therefore, it needs an integration test which may be beyond this PR's scope of work. Also I found there's no WebSocket related integration tests. If you think it's proper to add an WebSocket integration test in this PR, I'll take some time to do 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