sjvanrossum commented on code in PR #32060:
URL: https://github.com/apache/beam/pull/32060#discussion_r1751627440
##########
sdks/java/io/solace/src/main/java/org/apache/beam/sdk/io/solace/broker/BasicAuthJcsmpSessionServiceFactory.java:
##########
@@ -65,11 +64,15 @@ public abstract static class Builder {
@Override
public SessionService create() {
- return new BasicAuthJcsmpSessionService(
- checkStateNotNull(queue, "SolaceIO.Read: Queue is not set.").getName(),
- host(),
- username(),
- password(),
- vpnName());
+ BasicAuthJcsmpSessionService.Builder builder =
BasicAuthJcsmpSessionService.builder();
+ if (queue != null) {
+ builder = builder.queueName(queue.getName());
+ }
Review Comment:
Why did this change from throwing to checking?
Also, given that AutoValue implementations are immutable, deriving from a
class that allows you to mutate the queue name seems to run counter to what the
framework provides. SessionServiceFactory should probably be an interface
declaring create and optionally define an abstract class that implements the
interface (abstract of course) and declares equals and hashCode abstract if
your intention is to have all implementors explicitly override those methods
for value equality.
--
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]
