DanielLeens commented on PR #12156:
URL: https://github.com/apache/seatunnel/pull/12156#issuecomment-5713036609
@SEZ9 I can answer both procedural questions directly, since I just
re-checked this against the live PR rather than relying on the thread history.
**Head commit.** Confirmed via the API right now: the PR's commit list is
exactly `303a7574890b` -> `bf68705073ff` -> `4c25d9a62c68`, in that order, and
`4c25d9a62c68f9a7e13e404c1a81b3a3c56886de` is the current tip. So yes,
`bf6870507` is included - it's the immediate parent of the current head in that
same chain, not a separate/older line. `4f5a779a1` isn't part of this PR's
history at all; it doesn't appear in the three-commit list above, so
CryoThrust's earlier phrasing there was just confusing wording about an
unrelated commit, not a sign of two diverging heads to reconcile.
**Null-state path for F1.** I pulled `PhysicalPlan.java` at the confirmed
head (`4c25d9a62c68`) directly, not from memory:
```java
public void cancelJob() {
JobStatus jobStatus = getJobStatus();
if (jobStatus.isEndState()) {
...
return;
}
if (NOT_STARTED_STATUSES.contains(jobStatus)) {
```
`getJobStatus()` still just returns `(JobStatus)
runningJobStateIMap.get(jobId)`, so if that IMap entry is null, `jobStatus` is
null and `jobStatus.isEndState()` throws an NPE before either branch of the
not-started check is ever reached - same as `stopJob()`'s behavior today, and
same as `cancelJob()`'s behavior before this PR (the old ordinal check sat
behind the identical `isEndState()` guard). F1 never touched this path; what it
removed was the second, independent IMap read potentially observing a different
non-null status than the one `isEndState()` already validated. I don't think
this PR needs a dedicated null-path test, since that NPE behavior predates it
and isn't something F1's fix changed, but I wouldn't object to one landing as
extra coverage if you'd still like it.
--
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]
