mohitduhan19 commented on PR #4504: URL: https://github.com/apache/iggy/pull/4504#issuecomment-6162656119
@slbotbm thanks for the review. Pushed the following: - `tls_ca_file` is now supported. It maps to the existing `tlsCertificate(...)` setter. - `fromConnectionString` is now on both `IggyTcpClientBuilder` and `AsyncIggyTcpClientBuilder`, so `Iggy.tcpClientBuilder().async().fromConnectionString(...)` works. I added a login test for the async client. - An option that is set twice, like `?tls=true&tls=false`, is now rejected instead of the last value winning. - Added a test for an empty query (`...:8090?`), which is rejected. About `tls_domain`: the Java TCP client has no setting for the TLS server name today. The SSL handler always uses the connection host (`sslContext.newHandler(ch.alloc(), host, port)` in `AsyncTcpConnection`). Supporting it needs a new `tlsDomain(...)` builder option that is passed down to the connection. Do you want that in this PR, or in a follow-up? Until then `tls_domain` is rejected with a clear error. The parser and builder unit tests pass locally. CI will run the integration tests. /ready -- 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]
