wombatu-kun commented on PR #19265:
URL: https://github.com/apache/hudi/pull/19265#issuecomment-4954682262
@vinothchandar rewrote every inline comment as "what is broken / what to
change", and re-checked each one against the code by hand. One of my own
recommendations was wrong and is now corrected in place: `ChatOllama` has no
`timeout` field, so the ollama branch needs `client_kwargs={"timeout": ...}`,
not `timeout=`.
Worth fixing in this PR, all small:
- `build-jars.sh`: the JDK guard probes `java` from `PATH`, not
`$JAVA_HOME`, so the one-command quickstart always dies at the Trino-plugin
step on any machine whose default JDK is not 23.
- `llm.py`: the default provider (ollama) gets no timeout, so
`GATEWAY_LLM_TIMEOUT_SECONDS` is dead on the default deployment and a stalled
Ollama hangs `/v1/chat`.
- `spark-app.yaml`: `--spark-version 4.1` submits a job pointing at an image
that was never built. Agree that one version is enough here: drop the flag.
- `validate_source_copyright.sh`: a repo-wide `*.png` exemption for one
logo; data-URI as you said, then revert the line.
- `chat.py`: `is_error` is a substring test, so a successful result with a
column named `error` is reported as a failed tool call.
- `trino_tools.py`: `truncated` stays `false` for row-capped results, so the
agent presents a capped answer as the whole answer.
Follow-ups, not v1 blockers. I will file these as issues unless you would
rather fold them in:
- `trino_client.py`: a query timeout cancels neither the Trino query nor the
worker thread; repeated timeouts starve the shared executor.
- `/ready` runs an uncached `SELECT 1` per probe, and the readinessProbe has
no `timeoutSeconds` (k8s default 1s against the check's own 5s budget).
- guardrails: no test reaches the defense-in-depth `walk()`.
- README: state explicitly that v1 ships with no authn/authz.
--
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]
