SEZ9 opened a new pull request, #12629: URL: https://github.com/apache/seatunnel/pull/12629
## Benchmark first `benchmark/runner.py --provider bedrock --model us.anthropic.claude-haiku-4-5-20251001-v1:0 --preset smoke --level l1` (12 tasks, haiku-4-5): | run | pass@1 | misrouted | generation errors | |---|---|---|---| | `dev` | 11/12 (91.7%) | 0 | 0 | | this branch, run A | 6/12 (50.0%) | 0 | 0 | | this branch, run B | 11/12 (91.7%) | 0 | 1 | I am reporting both runs because they make the honest point: **this preset's pass@1 is not stable enough to read from a single run.** Those two runs are the same tree, and they differ by five tasks. At temperature 0.2 with 12 tasks one task is 8.3 points, so a single-run pass@1 delta below roughly 40 points carries no information. If I had only run it once I could have reported either "no change" or "a 42-point regression" from identical code, so I would rather say up front that pass@1 is a no-regression check here and nothing more. The signals that are actually stable, and that do matter for this change, are the two zero columns: **0 misroutes** (the planner still routes every config request to generation) and generation errors unchanged. Plus **219 tests pass** including 8 new ones. But the real evidence for a refactor is not a benchmark, it is that behaviour is unchanged at each call site, so the bulk of this PR is the test file. ## Purpose of this pull request Four places in `seatunnel-cli` talk to the Zeta REST API, and each one had written its own `urllib` call: `connectors.py` for the engine liveness probe and the option-rules fetch, and `cli.py` for job submit and status polling. They had drifted apart in ways that matter: - the submit path read `e.read()` on an `HTTPError` so the engine's rejection reason survived; the polling path did not, so a failure there became a bare status code - timeouts were 2s, 5s, 10s and 30s, each hardcoded at the call site with no indication of why - every response did `json.loads(resp.read())` with no bound, so a large `errorMsg` from the engine is read entirely into memory before anything looks at it - each site repeated its own `try/except URLError/HTTPError/JSONDecodeError` triangle This adds `rest.py` with two functions and moves all four call sites onto them: - `request_json(url, *, method, data, headers, timeout)` — raises `RestError` carrying `.status`, `.body` and `.url`. Reading the error body is now the single behaviour rather than a thing one call site remembered to do, which is what the failure-diagnosis path in #12628 depends on. - `is_reachable(url, *, timeout=2)` — the liveness probe. It never raises: any failure means "engine offline" and the caller falls back to offline metadata. Timeouts stay per-call-site, since 2s for a liveness probe and 30s for a job submit are both correct, but they are now arguments at the site instead of magic numbers inside duplicated plumbing. ### One intentional behaviour change `MAX_RESPONSE_BYTES = 8 MiB`. Responses above it raise `RestError` instead of being buffered. The previous unbounded `resp.read()` is unlikely to bite in practice, but the ceiling is cheap and 8 MiB is far above any legitimate `job-info` or option-rules payload. I am calling it out because it is the one case where the new code can fail where the old code would have succeeded. ### Dead code removed `agents.py` had a block under `if _check_engine():` that built a `url`, `headers` and `data` for `/submit-job`, then discarded all three and never sent a request — the `phase3_api` result was always `"SKIPPED (no dedicated validate endpoint)"`. Since there is still no validate endpoint, the block is now just that assignment plus a comment saying why. Rather than migrate code that never ran, I deleted it. ## Does this PR introduce _any_ user-facing change? No, other than the 8 MiB cap above. One message improves: a polling failure that previously showed only an HTTP status now shows the engine's reason, because that path gets the error-body read the submit path already had. ## How was this patch tested or verified? `tests/test_rest.py` runs against a real loopback `HTTPServer` rather than a patched `urlopen`, because the thing most worth protecting is actual `urllib` behaviour — that `HTTPError` is both an exception and a readable response, which is exactly the subtlety the old call sites disagreed on. Mocking `urlopen` would have asserted my own mock. 8 tests: GET JSON decode, POST body/headers/method reaching the server, HTTP error preserving status *and* body, oversized response refused, a response exactly at the limit still accepted, and the three `is_reachable` cases including nothing listening on the port. Full suite: 219 passed. ## Check list * [x] If any new Jar binary package adding in your PR, please add License Notice according [New License Guide](https://github.com/apache/seatunnel/blob/dev/docs/en/contribution/new-license.md) * [x] If necessary, please update the documentation to describe the new feature. https://github.com/apache/seatunnel/tree/dev/docs * [x] If you are contributing the connector code, please check that the following files are updated: 1. Update change log that in connector document. For more details you can refer to [connector-v2](https://github.com/apache/seatunnel/tree/dev/docs/en/connector-v2) 2. Update [plugin-mapping.properties](https://github.com/apache/seatunnel/blob/dev/plugin-mapping.properties) and add new connector information in it 3. Update the pom file of [seatunnel-dist](https://github.com/apache/seatunnel/blob/dev/seatunnel-dist/pom.xml) * [x] Update the [`release-note`](https://github.com/apache/seatunnel/blob/dev/release-note.md). -- 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]
