SEZ9 commented on PR #12299: URL: https://github.com/apache/seatunnel/pull/12299#issuecomment-5852038118
The automated summary above reports that every item from the previous round is addressed on `50567aa788b0`. That matches what I was hoping to see, but since it's a machine-generated write-up I'd like to close the loop against the actual diff before signing off. A few concrete confirmations from you would do it: **F1 / F3 / F4 – single snapshot, single decode path, truncation marker** - Please confirm `FileUtils` now returns a `FileTail` (retained bytes + size snapshot + truncated flag) and that `LogContentReader.read` no longer calls `Files.size(path)` separately, so the notice and the returned content can't drift apart. - Please confirm both the "fits in the limit" branch and the tail branch decode via UTF-8 – i.e. `readFileToStr` with the platform default is no longer on either path. - For the truncation marker: is the notice prefixed to the body for both the v2 and v1 endpoints (servlet and `RestHttpGetCommandProcessor`)? A quick sample of the notice text in the PR description would help reviewers and Web UI consumers alike. **F2 / F6 – compatibility record + v1 docs** - Please point me at the `incompatible-changes.md` (en/zh) entry and the "Response Size Limit" section in `rest-api-v1.md` (en/zh). If they're in the current head, just say so; if they landed as separate commits, a note on which ones is enough. **F7 – doc wording** - The "configured option, 64 MB by default" wording and the removal of the stray zh double backtick sound right; confirm both en/zh v2 pages were updated the same way. **F8 – consolidated conversion** - `HttpConfig.getLogResponseMaxSizeBytes()` as the single MB→bytes / `<= 0` = unlimited place, with `LogBaseServlet` and `LogService` delegating, is exactly what I asked for. Confirm no other copy of the conversion remains. **F5 – extra copies** - Going from the retained window straight into the notice-prefixed response without an intermediate full `String` addresses the concern. If you have a rough before/after peak-heap number for a single over-limit request, add it to the PR description; not blocking. Once you've confirmed the above (a short checklist reply is fine), I'll do a final pass over the head. Regarding CI, I don't want to rely on the summary's characterisation – please re-trigger the failing run or link to the failing job so we can judge for ourselves whether it's related. <!-- streview-comment:1345 --> -- 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]
