kennknowles commented on code in PR #35919:
URL: https://github.com/apache/beam/pull/35919#discussion_r2293866390
##########
sdks/java/io/solace/src/main/java/org/apache/beam/sdk/io/solace/read/UnboundedSolaceReader.java:
##########
@@ -181,11 +181,18 @@ public void finalizeReadyMessages() {
@Override
public Instant getWatermark() {
- // should be only used by a test receiver
if (getSessionService().getReceiver().isEOF()) {
return BoundedWindow.TIMESTAMP_MAX_VALUE;
}
- return watermarkPolicy.getWatermark();
+ Instant watermark = watermarkPolicy.getWatermark();
+ if (watermark == null) {
Review Comment:
> Oh well my pr was a follow up fix for my older pr that got closed due to
inactivity(#34296), this is the fix I thought @Abacn had asked for and since I
had gotten back to contributing I thought about putting some work into this :)
Thank you! Yes, I still think we should try to fix it. There is a null
getting in somewhere that should be impossible, and we can track it down.
> Also as you said that:
>
> > null checking is enabled for that class so the type should be accurate.
>
> since I am trying to learn about the codebase could you tell me how this
works.
Yes! We use [checkerframework](https://checkerframework.org/) to modernize
Java's type system. Any time you see a type `Foo` anywhere in the Beam codebase
this means it is a non-null `Foo`. Only `@Nullable Foo` can be null.
But because we added this after the project was somewhat large, many classes
have `@SuppressWarnings("nullness")` and that turns it off. But this is
forbidden for new code. The SolaceIO connector is in good shape; it does not
turn off the null checking.
So this `null` that violates the type system is probqably coming from one of
the following places:
- an external library that returns null but is not annotated that it may
return null,
- some part of the Beam codebase where null checking is disabled,
So the solution here is to figure out how we have a forbidden null. I read
the javadoc on `WatermarkPolicy.getWatermark()` and it seems pretty clear that
it should never return null. I clicked through some of the other methods that
could end up leaking a null into it and they all were autovalue classes that
also do not allow null, so I'm not sure!
--
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]
