sabbey37 commented on a change in pull request #6604:
URL: https://github.com/apache/geode/pull/6604#discussion_r650632179



##########
File path: 
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/publishAndSubscribe/Subscriptions.java
##########
@@ -90,11 +90,29 @@ boolean exists(Object channelOrPattern, Client client) {
 
     return findChannelNames()
         .stream()
-        .filter(name -> globPattern.matches(Coder.bytesToString(name)))
+        .filter(name -> globPattern.matches(Coder.bytesToString((byte[]) 
name)))

Review comment:
       The cast to `byte[]` here is unnecessary and can be removed.

##########
File path: 
geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/publishAndSubscribe/AbstractPubSubIntegrationTest.java
##########
@@ -161,7 +163,8 @@ public void 
channels_shouldOnlyReturnChannelsWithActiveSubscribers() {
   @Test
   public void 
channels_shouldNotReturnDuplicates_givenMultipleSubscribersToSameChannel_whenCalledWithoutPattern()
 {
 
-    Jedis subscriber2 = new Jedis("localhost", getPort(), JEDIS_TIMEOUT);
+    Jedis subscriber2 =
+        new Jedis("localhost", getPort(), 
AbstractPublishAndSubscribeIntegrationTest.JEDIS_TIMEOUT);

Review comment:
       There is a `BIND_ADDRESS` constant in `RedisClusterStartupRule` that 
could be used here and in other places in this class instead of the `localhost` 
string.  There is also a `REDIS_CLIENT_TIMEOUT` constant in that same class 
that could be used instead of `JEDIS_TIMEOUT` (I think we're trying to make 
this consistent throughout the module).




-- 
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:
[email protected]


Reply via email to