tkaymak commented on code in PR #39253:
URL: https://github.com/apache/beam/pull/39253#discussion_r3579861154
##########
sdks/java/io/jms/src/main/java/org/apache/beam/sdk/io/jms/JmsIO.java:
##########
@@ -484,6 +510,23 @@ public Read<T> withRequiresDeduping() {
return builder().setRequiresDeduping(true).build();
}
+ /** Specify the {@link AcknowledgeMode} used for consuming and
acknowledging JMS messages. */
+ public Read<T> withAcknowledgeMode(AcknowledgeMode acknowledgeMode) {
+ checkArgument(acknowledgeMode != null, "acknowledgeMode can not be
null");
+ return builder().setAcknowledgeMode(acknowledgeMode).build();
+ }
+
+ /**
+ * Specify the custom integer code for individual message acknowledgment
when using {@link
+ * AcknowledgeMode#INDIVIDUAL_ACKNOWLEDGE}.
+ *
+ * <p>Different JMS providers use different proprietary integer constants
for individual
+ * acknowledgment (e.g., ActiveMQ uses 4, Qpid JMS / ActiveMQ Artemis /
IBM MQ use 101).
Review Comment:
The `withIndividualAcknowledgeModeCode` javadoc says "Qpid JMS / ActiveMQ
Artemis / IBM MQ use 101". IBM MQ classes for JMS only support the standard
acknowledgment modes (AUTO=1, CLIENT=2, DUPS_OK=3) and have no individual
acknowledge constant, so 101 is not a valid mode there and
`createSession(false, 101)` on IBM MQ would throw a `JMSException` rather than
give individual ack.
Suggest dropping IBM MQ from that list. For per message behavior on IBM MQ
the guidance is `CLIENT_ACKNOWLEDGE`, which acks cumulatively. The "ActiveMQ
uses 4" part checks out against `ActiveMQSession.INDIVIDUAL_ACKNOWLEDGE`.
Minor, since `getAckModeCode` correctly throws for unrecognized providers, it
might be worth a one line note that `INDIVIDUAL` mode on other providers
requires an explicit `withIndividualAcknowledgeModeCode`.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]