featzhang opened a new pull request, #31:
URL: https://github.com/apache/flink-connector-http/pull/31
## What is the purpose of the change
The HTTP request timeout for the sink (`http.sink.request.timeout`) and
lookup source (`http.source.lookup.request.timeout`) were already supported at
runtime, but were not exposed as typed `ConfigOption` definitions in their
respective connector options classes.
This meant:
1. The options were not listed in `optionalOptions()` of the
`DynamicTableFactory` implementations — Flink could not validate them.
2. Users received a confusing "unsupported option" warning when configuring
them via SQL DDL.
3. There was no type safety, no default-value documentation, and no IDE
auto-completion support.
4. A `TODO` comment existed in `HttpLookupTableSourceFactory` noting this
gap.
This PR resolves the above by adding proper `ConfigOption<Duration>`
definitions and registering them in their factories.
## Brief change log
- Add `HttpLookupConnectorOptions.SOURCE_LOOKUP_REQUEST_TIMEOUT`
- Key: `http.source.lookup.request.timeout`
- Type: `Duration`
- Default: `30s`
- Add `HttpDynamicSinkConnectorOptions.SINK_REQUEST_TIMEOUT`
- Key: `http.sink.request.timeout`
- Type: `Duration`
- Default: `30s`
- Register both options in `optionalOptions()` of their respective
`DynamicTableFactory`
- Remove the `TODO` comment from `HttpLookupTableSourceFactory`
- Add unit tests in `HttpLookupTableSourceFactoryTest` and
`HttpDynamicTableSinkFactoryTest`
## Verifying this change
- New unit tests in `HttpLookupTableSourceFactoryTest`:
- `shouldAcceptWithRequestTimeout`: verifies that
`http.source.lookup.request.timeout=60s` is accepted without a
`ValidationException`
- `shouldUseDefaultRequestTimeoutWhenNotConfigured`: verifies that
omitting the option still creates the source successfully
- New unit test in `HttpDynamicTableSinkFactoryTest`:
- `requestTimeoutOptionTest`: verifies that
`http.sink.request.timeout=60s` is accepted as a valid option
All 11 affected tests pass. Full build with `mvn clean install -DskipTests
-Pfast` succeeds.
## Does this pull request potentially affect one of the following areas?
- **Dependencies**: No
- **Performance**: No
- **Security**: No
- **Breaking changes**: No — this is purely additive. The runtime behaviour
is unchanged; only the option declaration is formalised.
## Documentation
The `ConfigOption` definitions include `withDescription()` text describing
each option. Both options will appear in auto-generated connector documentation.
--
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]
