DanielLeens commented on PR #12858: URL: https://github.com/apache/seatunnel/pull/12858#issuecomment-6466880405
Thanks @SEZ9, and thanks @karthik-0306. I looked at the two open questions against the current head (`5c6b35f83`) so the next pass is quicker. **F3 (concurrency) and F4 (Windows).** Both are handled in `plugin_installer.py`. `install_plugins()` takes an exclusive `fcntl.flock` on `config/.plugin_config.lock` (`_exclusive_lock`, around line 207) for the whole snapshot, select, run and restore sequence. This is POSIX-only on purpose, and `install_plugins()` rejects Windows up front (`_is_windows()` check, around line 262) with a clear "Linux and macOS only" error before it touches `plugin_config`. That is why the lock test is skipped on Windows: the code path it exercises is never reachable there. The author's reason for not dispatching to `bin/install-plugin.cmd` is that the script does not propagate a failed download as a non-zero exit, so the rollback could not be trusted. That is a sound trade-off in my view. **F1, F2, F5, F7, F8.** The author's comment from 2026-10-26 maps these one by one to code and tests (absolute `seatunnel_home`, line classification identical to `install-plugin.sh`, version validation, restore only when the bytes changed, no hard-coded release in the hint). Karthik, it would help if you could also paste the test name next to each F-number in a reply to SEZ9's comment, so the review can close them without re-deriving. **CI.** The red `Test with Python 3.9` check is the runner-image problem that is already fixed on `dev` by #12994 (pin to `ubuntu-24.04`). The minimum-cost next step is to rebase this branch onto the latest `dev` and let the check re-run; no extra change is needed in this PR. I have not changed my view of the code, and I will re-review as soon as a new commit lands. -- 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]
