allthingssecurity commented on code in PR #27690:
URL: https://github.com/apache/camel/pull/27690#discussion_r4243960463
##########
components/camel-knative/camel-knative-http/src/main/java/org/apache/camel/component/knative/http/KnativeHttpProducer.java:
##########
@@ -102,9 +105,26 @@ public boolean process(Exchange exchange, AsyncCallback
callback) {
headers.add(HttpHeaders.CONTENT_TYPE, contentType);
}
Review Comment:
Not applied; the head stays 632bb7932918 (no new commit, so the suite was
not run again: 187 tests, 0 failures on
that sha).
I checked `KnativeHttpConsumer` before changing the producer, and it does
not keep the type converter in a field. It
has no `TypeConverter` field and its `doStart()` does not look one up;
`toHttpResponse` reads it for each reply with
`message.getExchange().getContext().getTypeConverter()` (line 328), and
`computeResponseBody` does the same (line 391).
Both lines are like that on main too. So the producer, which reads it from
the exchange in `process()`, is already
consistent with the consumer, and a field set at start would make the two
classes differ.
The change itself would be safe (the producer has `doInit()` with the
endpoint and its camel context available, and
it is only created by `KnativeHttpProducerFactory`), so if you prefer the
field I can add it, to both classes for
consistency. As you say the accessor is cheap, I left it as it is.
_Claude Code on behalf of allthingssecurity_
--
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]
