oscerd commented on PR #3085:
URL: https://github.com/apache/camel-kamelets/pull/3085#issuecomment-5994469424
Read through this and checked the three claims in the description rather
than taking them on trust. All three hold. One small nit below, and nothing
blocking.
## Verified: unset options really are dropped
The description says "the existing sink routes keep passing, because both
options are forwarded only when set" — that is the load-bearing claim, since
`modality` and `contentType` do not exist on the component until CAMEL-25027
lands. Probed the mechanism directly with a throwaway Kamelet forwarding a
deliberately bogus option through `{{?...}}`:
**Unset** — the parameter is dropped entirely, route starts, message
processed:
```
exit=0 (log line present, no endpoint error)
```
**Set** — it reaches the endpoint and Camel rejects it:
```
ResolveEndpointFailedException: Failed to resolve endpoint:
log://probe?noSuchComponentOption=audio/wav
due to: There are 1 parameters that couldn't be set on the endpoint.
Unknown parameters=[{noSuchComponentOption=audio/wav}]
```
So existing `langchain4j-ingest-sink` routes are genuinely unaffected on a
Camel without the options, and anyone who sets `modality` before CAMEL-25027 is
in the snapshot gets a loud route-creation failure naming the parameter rather
than a silently ignored setting. That is the right failure mode, and worth
having in the record.
## Verified: the lowercase enum is fine
The description notes the component lists `TEXT`/`MEDIA` while the Kamelet
declares `["text", "media"]`, on the grounds that Camel converts enum options
case-insensitively. Confirmed on 4.22.0 against `log:`, which has the same
shape (`LoggingLevel`):
- `log:lower?level=info` → starts, exit 0
- `log:lower?level=nosuchlevel` → `No enum constant
org.apache.camel.LoggingLevel.NOSUCHLEVEL`
The uppercased name in the error shows Camel normalising before the lookup,
which is the behaviour being relied on. Lowercase also matches the rest of the
catalog, so this is the right call.
## One nit: the contentType pattern is unanchored
```yaml
pattern: "[^/ ;]+/[^/ ;]+(;.*)?"
```
JSON Schema `pattern` is a partial match, so this accepts anything
*containing* something MIME-shaped:
```
MATCH 'audio/wav'
MATCH 'text/plain; charset=UTF-8'
MATCH 'garbage audio/wav garbage' <-- accepted
MATCH ' audio/wav ' <-- accepted (leading/trailing space)
reject 'not-a-mime'
```
Anchoring gives what it looks like it already does:
```yaml
pattern: "^[^/ ;]+/[^/ ;]+(;.*)?$"
```
```
MATCH 'audio/wav'
MATCH 'text/plain; charset=UTF-8'
reject 'garbage audio/wav garbage'
reject ' audio/wav '
```
The catalog is consistent on this where it matters — all 27 of the
URI-scheme property patterns are `^`-anchored. It is typo containment rather
than a trust control, so entirely your call, but the leading/trailing-space
case seems worth catching since a stray space in a YAML property is an easy
mistake.
## Other notes
- Draft status and the dependency on apache/camel#27289 are set correctly,
and `no checks reported` on the branch is consistent with that rather than a CI
problem.
- The `maxDocumentSize` / `minDocumentSize` description changes
("characters, bytes with modality media") are a good catch — those would
otherwise silently mean something different in media mode.
- The `langchain4j-ingest-file-source` charset note pointing at media mode
is the kind of cross-reference that is easy to omit; good that it is here.
I have not run the new Citrus test, since it needs the unmerged component
change. Happy to re-check once CAMEL-25027 is in the snapshot.
---
_Claude Code on behalf of Andrea Cosentino_
--
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]
