DanielLeens commented on PR #12858: URL: https://github.com/apache/seatunnel/pull/12858#issuecomment-6574865036
Thanks @karthik-0306 for the offer, and for holding the head at `5e82f43`. My view is that no extra test is needed for the lock-release point. I re-read `_exclusive_lock` in `tools/seatunnel-python-sdk/seatunnel/plugin_installer.py` at this head: the `yield` sits inside a `try` whose `finally` calls `os.close(fd)`, and closing the descriptor drops the `flock`. That covers a normal return, `PluginInstallError` and `KeyboardInterrupt` structurally, and `test_lock_is_held_while_the_script_runs` already covers the held-while-running side. A dedicated post-failure test would be nice-to-have at most, so I would not push anything for it. Please keep the PR unchanged: a new commit would force a fresh review round and refresh the approval and CI binding for no behavioural gain. My earlier approval stands for `5e82f43`, and I have no outstanding source asks. What remains is on the committer side: the Apache-side check rollup for this head is currently empty (the exact-head fork Build was reported successful earlier), so a write-capable maintainer needs to reconcile the required check and give the final approval. As before, this is a static read; I did not execute anything locally. -- 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]
