kuswardhanietidims-svg commented on PR #11997:
URL: https://github.com/apache/seatunnel/pull/11997#issuecomment-5463051387
Thanks for the thorough re-review. I've addressed all three non-blocking
items in commit `9be70636c`:
**Issue 1 — `ORCAROUTER_ECHO_REASONING_CONTENT` env var (parity with
`OpenAIProvider`)**
`OrcaRouterProvider.__init__` now reads `ORCAROUTER_ECHO_REASONING_CONTENT`
via the existing `_env_bool()` helper, defaulting to `true` (same default as
`OPENAI_ECHO_REASONING_CONTENT`):
```python
self._echo_reasoning_content =
_env_bool("ORCAROUTER_ECHO_REASONING_CONTENT", True)
```
Added a unit test (`test_echo_reasoning_content_env_override`) covering
`false`, `true`, and the unset-default path.
**Issue 2 — redundant `if not os.environ.get(...)` guard before
`setdefault`**
Dropped the existence checks in `create_provider()` for both
`ORCAROUTER_MODEL` and `ORCAROUTER_SMALL_FAST_MODEL`; the bare
`os.environ.setdefault(...)` now matches the tighter form used elsewhere in the
file.
**Issue 3 — temp dir leak in `test_config_model_override_is_independent`**
Switched from `tempfile.mkdtemp()` to `tempfile.TemporaryDirectory()` with
`tmp.cleanup()` in the existing `finally` block.
The new `ORCAROUTER_ECHO_REASONING_CONTENT` var is also documented in
`env.example.sh`, `README.md`/`README.zh-CN.md` (env-var reference table), and
the `docs/en+zh/ai-cli/quickstart.md` examples.
Verification: `python3 -m pytest tests/` → 83 passed (was 82), all existing
tests green, no new dependencies. The branch is rebased on latest `dev` and
force-push was not needed for this commit (it's an additive commit on top of
`4a576f78e`).
--
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]
