CryoThrust commented on issue #11007: URL: https://github.com/apache/seatunnel/issues/11007#issuecomment-6137859172
Following the latest note that InfluxDB is unclaimed, I audited a narrow timeout-validation slice rather than adding new restrictions to all numeric options. `InfluxDBClient.getInfluxDB` passes `connect_timeout_ms` to OkHttp in milliseconds and `query_timeout_sec` in seconds. The native builder rejects negative values and durations exceeding `Integer.MAX_VALUE` milliseconds, while both source and sink `optionRule()` currently accept them. A factory check therefore succeeds and the job fails later when creating the client. The proposed rules mirror those existing bounds: | Option | Inclusive range | Meaning of zero | | --- | --- | --- | | `connect_timeout_ms` | 0–2147483647 | no connect timeout | | `query_timeout_sec` | 0–2147483 | no read timeout | Both remain optional with unchanged defaults. Only the two factory rules change; HTTP client creation, connectivity, credentials, format handling and runtime checks stay intact. I would leave batch/retry semantics out of this migration: those need a separate compatibility discussion rather than inventing bounds for previously accepted values. A local prototype extends the existing `InfluxDBFactoryTest`. With otherwise valid source/sink configurations, eight cases pass the native-builder rejection assertion but fail because the original factory accepts the invalid timeout; defaults, zero, positive values and inclusive upper limits remain valid. After applying the rules, the complete connector module has 51 passing tests with no skips. EN/ZH source/sink docs explain the ranges, and the existing E2E fixture now explicitly uses zero for both source and sink to preserve that behavior. The live engine/InfluxDB E2E has not been run locally. I found no open InfluxDB timeout-validation PR. #12741 handles retention/null-time/flush behavior and #12744 handles generated timestamp units, so this is independent of those changes. There was an older claim by @TianHengZhuang; if that work is still active, I can hand over the timeout tests rather than duplicate it. Does this two-option source/sink slice fit the current migration scope? I can open the focused PR here once ownership and scope are clear. -- 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]
