nzw921rx commented on PR #11791: URL: https://github.com/apache/seatunnel/pull/11791#issuecomment-5409681783
Thanks @SEZ9 for the detailed review. I rechecked each item against the current head and kept this follow-up focused on the concrete runtime risk. The latest commit is [`9874c10b2`](https://github.com/apache/seatunnel/pull/11791/commits/9874c10b268a99d3eca22534eb859007bd65f984): - [x] **Issue 5:** all eight updated-module shard steps now pass `it-modules` through `IT_MODULES` in the step environment and consume `"$IT_MODULES"`. The value is no longer expanded directly into the shell script. - [x] **Issue 3:** the workflow now points maintainers to `tools/update_modules_check/update_modules_check.py`, where the dedicated-job ownership rule, historical timing source, seed-selection procedure, and regression-test update are documented. For the other items, this is the code-path confirmation checklist I used: - [x] **Issues 1 and 4:** `modules_to_json()` removes the Maven `:` prefix and emits bare artifact IDs; empty input is exactly `[]`. `test_modules_to_json_preserves_exact_module_tokens` pins that contract, and `test_dedicated_job_conditions_match_json_module_tokens` cross-checks the dedicated workflow conditions. - [x] **Issue 2:** `ut-modules` is only used as the non-empty guard for the `unit-test` job. Both Linux and Windows steps run the full reactor and do not pass `ut-modules` to Maven `-pl`, so there is no JSON-to-`-pl` consumer on this path. - [x] **Issue 6:** `seatunnel-edge-agent-e2e` is treated as an optional module owned by a dedicated job and is explicitly removed by `get_sub_update_it_modules`; the ownership and shard-boundary tests cover this path. - [x] **Issue 7:** the seed re-tuning procedure is documented next to the seed. The latest full validation run completed every shared connector shard below the 150-minute timeout; the slowest was about 101 minutes: https://github.com/nzw921rx/seatunnel/actions/runs/32751432790 - [x] **Issue 8:** the producer uses compact, single-line `json.dumps()` output. The exact-output tests pin both the empty and non-empty forms, and the helper has no diagnostic output on stdout. Local checks for this follow-up passed: - 29 Python unit tests - workflow YAML parse (55 jobs) - `git diff --check` - `./mvnw spotless:apply` The GitHub Build check for the current head has also been queued. Could you please confirm whether this addresses the requested changes? If I missed a concrete consumer or execution path, I am happy to follow up with another focused adjustment. -- 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]
