SEZ9 commented on PR #12858:
URL: https://github.com/apache/seatunnel/pull/12858#issuecomment-6552250987

   Thanks for the docstring-only push, @karthik-0306 — understood that 
`install_plugins`' docstring now states `select_plugins()` works on any 
platform, that it matches the Windows error message, and that the tests are 
unchanged. The +1 line shift from L262 onward is noted, so the F1-F8 references 
still line up on my side.
   
   Before I can do the final pass I need one thing from you: a short 
per-finding status for F1-F8, since this comment only tells me about the 
docstring and I don't want to assume the rest. Concretely:
   
   - **F1** (relative `seatunnel_home` resolving the script path twice under 
`cwd=seatunnel_home`) — addressed, or still open?
   - **F2** (`select_plugins` leaving non-fully-matching connector lines 
enabled) — addressed, or still open?
   - **F3** (no inter-process lock around snapshot → select → run → restore) — 
addressed, deferred, or still open?
   - **F4** (hard `bash` + `bin/install-plugin.sh` requirement vs. 
`bin/install-plugin.cmd` on Windows) — the docstring tweak clarifies the 
contract, but does the code now avoid picking up a WSL/Git `bash.exe` and 
failing clearly on Windows, or is it still a documented limitation?
   - **F5** (`version` only checked for emptiness) — addressed, or still open?
   - **F6** (`UnicodeDecodeError` / `TypeError` leaking instead of 
`PluginInstallError`) — addressed, or still open?
   - **F7** (unconditional rollback write of `original`) — addressed, or still 
open?
   - **F8** (`'3.0.1'` example version in the error hint) — addressed, or still 
open?
   
   A one-liner per item ("fixed", "won't fix because …", or "open") is enough. 
If any are fixed, please point me to where so I can verify them directly rather 
than guess. Once I have that list I'll go through the current state of 
`tools/seatunnel-python-sdk/seatunnel/plugin_installer.py` and close out 
whatever is resolved.
   
   <!-- streview-comment:2178 -->


-- 
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]

Reply via email to