DanielLeens commented on PR #12740: URL: https://github.com/apache/seatunnel/pull/12740#issuecomment-6210081966
Thanks @SEZ9 for the question, and thanks @karthik-0306 for the quick turnaround. I checked the CI wiring on current `dev` so you do not have to guess. On the second point (do the new detector tests run in CI): as far as I can tell from the workflow files, no. The only Python workflow is `.github/workflows/cli.yml`, which is triggered by `seatunnel-cli/**` and runs `ruff` and `pytest` inside `seatunnel-cli` only. There is no workflow that references `tools/seatunnel-python-sdk`, so neither `tests/test_java_detector.py` from this PR nor the existing `tests/test_sdk.py` is executed by GitHub CI today. The `Build` check on this PR therefore says nothing about the Python tests (this is from reading the workflows on `dev`; I did not run anything). My suggestion is to keep that out of this PR: wiring a path-filtered pytest job for the SDK is a separate CI change that also affects the existing SDK tests, and the detector has already been verified locally on Windows and macOS (thanks @CryoThrust). It would fit well together with the launcher follow-up for #12707, or as a small dedicated PR. On the first point (`Build` red): the only failed job is still `Run / engine-v2-it (11, ubuntu-latest)`, in the first attempt of your fork's run (run attempt 1, so it has not been re-run yet). Since this PR only touches two Python files, that Java engine lane is not exercised by it, so please re-run just that failed job from your fork's Actions page (Re-run failed jobs). No code change is needed. My technical approval still stands. -- 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]
