DanielLeens commented on PR #10678: URL: https://github.com/apache/seatunnel/pull/10678#issuecomment-5287967546
@knight6236 I want to respond directly to your reasoning, since the docs item is the one thing I'm holding open on this head. I checked @davidzollo's precedent claim against the actual `dev` tree rather than taking it on faith, since that's the kind of thing worth confirming rather than assuming: - `docs/en/engines/zeta/engine-jar-storage-mode.md` documents `connector-jar-storage-enable` (default `false`) behind a `:::caution warn` block that reads almost exactly like the caveat you'd write for this feature: *"this feature is currently in an experimental stage, and there are many areas that still need improvement... we recommend exercising caution when using this feature to avoid potential issues and unnecessary risks."* - `docs/en/engines/zeta/rest-api-v1.md` documents a deprecated, disabled-by-default API surface the same way — `:::caution warn`, "disabled by default," here's exactly how to turn it on if you choose to. Both check out. So this isn't a hypothetical "maybe the project would document it this way" — it's the pattern already used twice for switches in the same state yours is in: off by default, real known caveats, not fully hardened. I don't think your underlying concern is wrong, to be clear — "will documenting this cause someone to flip it on before it's ready" is a legitimate question to ask before adding user-facing docs for a risky opt-in switch, and I'd rather you ask it than not. Where I land differently is on the mitigation. Silence doesn't reduce the actual risk to a user who enables `SEATUNNEL_CLASSLOADER_DEEP_CLEAN` — it only changes who finds out about it and how well-informed they are when they do. Concretely: the WARN log this PR already ships is not neutral on that front. It fires mid-failure and hands the operator two specific `--add-opens` flags to add, with none of the surrounding context — no mention of Phase 1/2 status, no mention of the JDK 8 `useCaches` tradeoff, no "this isn't guaranteed safe for cross-job JAR sharing until Phase 3." An ops runbook that captures "add these two JVM flags" from that log message, without the caveats, is a worse outcome than a docs paragraph that state s the caveats up front. That's the actual middle ground I'd ask for, and it's a smaller ask than a "how to use this in production" guide: one caution-boxed paragraph in `docs/en`/`docs/zh`, matching the two precedents above — - property name and default (`false`) - both required `--add-opens` flags - one line stating this is experimental, may change, and is not yet safe to rely on for cross-job JAR sharing until Phase 3 lands That's the same "experimental, subject to change" framing you already wrote out in your own comment above — I'm only asking for it to live in the docs next to the property name instead of only in this thread. It doesn't require touching the code, it doesn't change the default, and it doesn't ask you to write the "recommendations for production usage" section you're deferring to the Phase 3 doc PR — that part can absolutely wait. Separately, since this is a new opt-in switch with default unchanged, I don't think it needs an entry in `docs/en/introduction/concepts/incompatible-changes.md` — that file is for breaking/behavior changes on the default path, and this one is default-off by design, so I'm not adding that as a requirement here. On process: I'll note I don't have merge authority here — my read-level review can't gate this either way, same as @davidzollo said. But from my side, the docs paragraph above is genuinely the only thing I'm holding open; everything else on this head (the JDK 9+ `--add-opens` fix, the JDK 8 `useCaches` scoping, the static-flag-to-instance-field change, CI) is confirmed resolved. Happy to approve as soon as that paragraph lands, and equally happy to see a maintainer with write access weigh in if the two of you want to settle this a different way. -- 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]
