-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade Pulsar client (3.2.2) and fix JMS priority on consumer side using Pulsar 3.x #119
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I left some style comments
...jms/src/main/java/com/datastax/oss/pulsar/jms/MessagePriorityGrowableArrayBlockingQueue.java
Outdated
Show resolved
Hide resolved
...jms/src/main/java/com/datastax/oss/pulsar/jms/MessagePriorityGrowableArrayBlockingQueue.java
Show resolved
Hide resolved
...jms/src/main/java/com/datastax/oss/pulsar/jms/MessagePriorityGrowableArrayBlockingQueue.java
Outdated
Show resolved
Hide resolved
pulsar-jms/src/main/java/com/datastax/oss/pulsar/jms/PulsarConnectionFactory.java
Show resolved
Hide resolved
TCK Failure (suite with server side filters) `FAILED........com/sun/ts/tests/jms/core20/jmscontexttopictests/Client.java#createSharedDurableConsumerTest1_from_standalone`` TCK failure (suite with client side filters)
|
@@ -81,7 +81,7 @@ public void testAUTO_ACKNOWLEDGE() throws Exception { | |||
} | |||
|
|||
try (MessageConsumer consumer = session.createConsumer(destination); ) { | |||
assertEquals("foo", consumer.receive().getStringProperty("test")); | |||
assertEquals("foo", consumer.receive(com.datastax.oss.pulsar.jms.utils.PulsarCluster.DEFAULT_RECEIVE_TIMEOUT).getStringProperty("test")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is changing the test, why ?
receive() is different than receive(timeout)
@@ -92,12 +98,12 @@ public void testPulsar211Transactions() throws Exception { | |||
|
|||
@Test | |||
public void testPulsar3Transactions() throws Exception { | |||
test("apachepulsar/pulsar:3.0.0", true); | |||
test("apachepulsar/pulsar:3.2.2", true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3.2.2 is buggy, maybe we can use 3.2.1 if tests don't pass
@@ -106,11 +112,22 @@ public void testLunaStreaming210Transactions() throws Exception { | |||
test(LUNASTREAMING, true); | |||
} | |||
|
|||
@Test | |||
public void testLunaStreaming31Transactions() throws Exception { | |||
// waiting for Apache Pulsar 2.10.1, in the meantime we use Luna Streaming 2.10.0.x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can drop this comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
This reverts commit 1f1776b.
Other notes: