karthik-0306 commented on PR #12858:
URL: https://github.com/apache/seatunnel/pull/12858#issuecomment-6554330612
Thanks @SEZ9. Per-finding status at the current head 5e82f43 (all against
tools/seatunnel-python-sdk/seatunnel/plugin_installer.py). All eight are fixed;
none is open or deferred.
- F1 — fixed. seatunnel_home is resolved once to an absolute path (L287),
and bash gets that absolute script path with cwd=seatunnel_home, so the path
can't be resolved twice. Tests:
test_relative_home_is_resolved_to_an_absolute_script_path (L460),
test_invalid_seatunnel_home_raises_plugin_install_error (L348).
- F2 — fixed. select_plugins (L91-170) classifies lines the way
install-plugin.sh does. Blank lines, "-" headers and "#" comments are skipped.
Any other line must be a valid artifact ID, otherwise it raises with the line
number before anything is written (L148-153), so a connector line with a
trailing comment is no longer left enabled. Tests:
test_connector_lines_with_stray_whitespace_are_recognized (L192),
test_disabled_connector_with_trailing_whitespace_can_be_enabled (L206),
test_line_rejected_by_install_script_raises_and_leaves_file_unchanged (L213).
- F3 — fixed. _exclusive_lock (L207-227) takes an flock on
config/.plugin_config.lock. install_plugins holds it (L301) across snapshot,
select, run and restore. select_plugins() alone takes no lock; that is
documented in the module docstring. Test:
test_lock_is_held_while_the_script_runs (L501).
- F4 — fixed in code, not only documented. Windows is rejected at L263 with
a clear error that points to select_plugins(). This happens before
shutil.which("bash") at L297, so a WSL or Git bash.exe is never picked up. The
docstring (L233-236) now says the same thing and explains why
install-plugin.cmd can't be used: it does not report download failures through
its exit code, which would defeat the rollback. Test:
test_windows_is_rejected_before_rewriting (L475).
- F5 — fixed. The version is checked for emptiness (L270) and against the
script's allowed characters [A-Za-z0-9._-] (L274), before anything is
rewritten. Tests: test_empty_version_raises (L437),
test_invalid_version_raises_before_rewriting (L449).
- F6 — fixed. UnicodeDecodeError and OSError when reading the file (L120), a
non-iterable connectors argument (L67-72), a non-string connector name
(L76-77), and a non-string version (L269) all raise PluginInstallError. Tests:
test_non_utf8_file_raises_plugin_install_error (L225),
test_invalid_connectors_argument_raises_plugin_install_error (L234).
- F7 — fixed. The rollback goes through _restore_if_changed (L191-199,
called at L328), which writes only when the current bytes differ from the
snapshot. Test: test_failed_install_does_not_rewrite_an_unchanged_file (L486).
- F8 — fixed. The empty-version and invalid-version errors no longer name a
version; they say it should match your SeaTunnel engine version (L271-277).
Test: test_empty_version_raises (L437) asserts "engine version" is in the
message and "3.0.1" is not.
Related tests (rollback on failure, interrupt, restore failure): L294, L303,
L317, L326, L333, L372, L382.
--
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]
