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]