SEZ9 commented on PR #12858: URL: https://github.com/apache/seatunnel/pull/12858#issuecomment-6513455278
Thanks for the follow-up, @karthik-0306. **F4** — the rationale you describe (a `bin/install-plugin.cmd` that can't take part in the rollback, and a WSL/Git `bash` being handed a Windows path) sounds reasonable, and an explicit error that points to `select_plugins()` as the cross-platform alternative is the behaviour I'd expect. I can't confirm the docstring, the error message or `test_windows_is_rejected_before_rewriting` from this thread alone, so I'll verify against the diff. One small ask meanwhile: please keep the docstring text and the raised error message saying the same thing (Linux/macOS only, `select_plugins()` anywhere) so they don't drift. **F7** — comparing the current bytes against the snapshot and only writing when they differ is what I was after, and `test_failed_install_does_not_rewrite_an_unchanged_file` sounds like the right case to cover. Same caveat: I'll mark it resolved once I've seen `_restore_if_changed` in the diff. The table got cut off again, this time at `test_nonzero_e` in the F7 tests column. Could you repost the rest? I still need the Fix / Code / Tests rows for F1 (relative `seatunnel_home` + `cwd` double-resolution), F2 (connector lines with stray whitespace or trailing comments), F3 (inter-process lock), F5 (`version` validation before the script starts), F6 (`UnicodeDecodeError` / `TypeError` leaking from `select_plugins` / `_normalize_connectors`) and F8 (the `'3.0.1'` example in the error hint). Posting it as a code block or a short bulleted list might avoid the truncation. Once I have those I'll go through everything in one pass. <!-- streview-comment:2131 --> -- 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]
