bharathgunapati commented on code in PR #52:
URL:
https://github.com/apache/flink-connector-http/pull/52#discussion_r3523661159
##########
flink-connector-http/src/main/java/org/apache/flink/connector/http/sink/httpclient/AbstractRequestSubmitter.java:
##########
@@ -53,11 +55,14 @@ public AbstractRequestSubmitter(
"http-sink-client-response-worker",
ThreadUtils.LOGGING_EXCEPTION_HANDLER));
- this.httpRequestTimeOutSeconds =
- Integer.parseInt(
- properties.getProperty(
-
HttpConnectorConfigConstants.SINK_HTTP_TIMEOUT_SECONDS,
- DEFAULT_REQUEST_TIMEOUT_SECONDS));
+ // Sink runtime is wired with Properties today; reading
SINK_REQUEST_TIMEOUT from
+ // ReadableConfig will be handled in a follow-up sink
config-unification PR.
+ String requestTimeout =
Review Comment:
Heads-up on scope: this PR fixes `request.timeout` parsing on the sink path
(`TimeUtils.parseDuration(...)` instead of `Integer.parseInt(...)`), but the
value is still sourced from the `Properties` here rather than from the typed
`SINK_REQUEST_TIMEOUT` option via `ReadableConfig`.
That asymmetry is intentional for this bug fix — the sink runtime submitter
is
only handed `Properties` today (`RequestSubmitterFactory#createSubmitter`).
Reading `SINK_REQUEST_TIMEOUT` from `ReadableConfig` and threading typed
config
through `HttpSinkInternal` → `RequestSubmitterFactory` →
`AbstractRequestSubmitter`
will be handled in a separate sink config-unification follow-up PR.
--
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]
