SEZ9 commented on PR #12169:
URL: https://github.com/apache/seatunnel/pull/12169#issuecomment-5747502441
@JeremyXin thanks for the follow-up and for the clear summary of what
changed. I've gone through the latest head (`dfcc9b1369`) and the automated
review notes on it.
Where things stand from my side:
- The documentation/test items from the previous round (homogeneous
`dynamic-slot` assumption, tag-filter mismatch exclusion from shortage
evidence, the new regression test, and the `isValidAt()` Javadoc pointer to
`AutoscalerView#getLatestEvaluationRecord()`) look good and match what you
described.
- `dfcc9b136` ("run spotless:apply") resolves the only new problem flagged
on `814224b426` – the `recordShortage()` Javadoc wrapping in
`ResourceRequestHandler.java`. `Run / Code style` is reported green on this
head, so the downstream jobs are now actually executing instead of being
skipped.
- Keeping the edge-triggered recommendation and the disabled periodic repeat
unchanged is fine with me; the duplicate-downstream-action argument is sound
for an advisory-only slice.
Remaining asks before I approve:
1. The automated review raised one carryover item: `startAutoscaler()` is
not isolated from active-master activation. It's flagged as low severity and
non-blocking, but I'd like either a short explanation in this thread of why
it's safe as-is, or a small guard so a failure inside `startAutoscaler()` can't
affect master activation. Your call which – just let me know.
2. Please rerun the unrelated failing integration jobs so the final Build
status on `dfcc9b1369` is meaningful, and make sure the explicit Awaitility
timeouts and the null-field regression test stay in the final head.
3. If you push anything further, a one-line note on what changed would help
me avoid another full re-read.
Once (1) and (2) are sorted I'm happy to move this forward.
<!-- streview-comment:1190 -->
--
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]
