You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@iggy.apache.org by "chiradip (via GitHub)" <gi...@apache.org> on 2025/10/07 04:00:50 UTC

[PR] feat(sdk): Java SDK - :builder pattern for client #2202 [iggy]

chiradip opened a new pull request, #2245:
URL: https://github.com/apache/iggy/pull/2245

   Implement Builder Pattern for IggyClient Configuration
   
     Fixes #2202
   
     Changes
   
     Implemented builder pattern for both blocking and async Java TCP clients to provide a cleaner, more flexible API for client configuration.
   
     Modified Files:
     - IggyTcpClient.java - Added Builder and RetryPolicy inner classes
     - AsyncIggyTcpClient.java - Added Builder and RetryPolicy inner classes
   
     Features:
     - Fluent builder API with method chaining
     - Auto-login when credentials are provided
     - Configuration options: host, port, credentials, connection timeout, request timeout, connection pool size, retry policy
     - Default values: localhost:8090
     - Input validation for host and port
     - Full backward compatibility with existing constructors
   
     Example Usage:
     IggyTcpClient client = IggyTcpClient.builder()
         .host("localhost")
         .port(8090)
         .credentials("iggy", "iggy")
         .connectionTimeout(Duration.ofSeconds(30))
         .requestTimeout(Duration.ofSeconds(10))
         .retryPolicy(IggyTcpClient.RetryPolicy.exponentialBackoff())
         .build();
   
     Testing
   
     - Added comprehensive test suites: IggyTcpClientBuilderTest (14 tests) and AsyncIggyTcpClientBuilderTest (16 tests)
     - All 30 tests pass against live Iggy server
     - Validated builder functionality, auto-login, input validation, and backward compatibility


-- 
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: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(sdk): Java SDK - :builder pattern for client #2202 [iggy]

Posted by "hubcio (via GitHub)" <gi...@apache.org>.

hubcio commented on PR #2245:
URL: https://github.com/apache/iggy/pull/2245#issuecomment-3375580075

   @changhc agreed. commit title should be simple, e.g. `feat(java): implement builder pattern for client`
   @chiradip please fix


-- 
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: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(sdk): builder pattern for client [iggy]

Posted by "chiradip (via GitHub)" <gi...@apache.org>.

chiradip commented on PR #2245:
URL: https://github.com/apache/iggy/pull/2245#issuecomment-3376866887

   > @changhc agreed. commit title should be simple, e.g. `feat(java): implement builder pattern for client` @chiradip please fix, and do the same for all future PRs in Java SDK
   
   Done.


-- 
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: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(sdk): implement builder pattern for client [iggy]

Posted by "hubcio (via GitHub)" <gi...@apache.org>.

hubcio merged PR #2245:
URL: https://github.com/apache/iggy/pull/2245


-- 
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: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(sdk): Java SDK - :builder pattern for client #2202 [iggy]

Posted by "chiradip (via GitHub)" <gi...@apache.org>.

chiradip commented on PR #2245:
URL: https://github.com/apache/iggy/pull/2245#issuecomment-3375118347

   @mmodzelewski please check this minor changes 


-- 
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: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(sdk): implement builder pattern for client [iggy]

Posted by "chiradip (via GitHub)" <gi...@apache.org>.

chiradip commented on PR #2245:
URL: https://github.com/apache/iggy/pull/2245#issuecomment-3392719399

   > The builders for the async and blocking clients are duplicated. It would be good to have a common implementation since most (if not all) options will be the same. Also, the builder currently includes options that aren’t supported by the clients. I think we should keep the builder minimal for now and only add new options once the corresponding implementation is in place.
   
   --- Agreed. There are other duplication between async and blocking clients like https://github.com/apache/iggy/issues/2224, I think its better to address that later at once, or at similar time period. As far as unsupported parameters in builder, if you re talking about connection pooling, I am going to work on right away. Also they are optional. I would like to reduce the cycle of going back and forth to support connection pooling and coming back to support that in builder pattern. But I agree that optional stuff needs to be supported soon. 


-- 
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: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(sdk): implement builder pattern for client [iggy]

Posted by "chiradip (via GitHub)" <gi...@apache.org>.

chiradip closed pull request #2245: feat(sdk): implement builder pattern for client
URL: https://github.com/apache/iggy/pull/2245


-- 
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: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(sdk): implement builder pattern for client [iggy]

Posted by "chiradip (via GitHub)" <gi...@apache.org>.

chiradip commented on PR #2245:
URL: https://github.com/apache/iggy/pull/2245#issuecomment-3389908132

   Done 


-- 
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: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(sdk): builder pattern for client [iggy]

Posted by "hubcio (via GitHub)" <gi...@apache.org>.

hubcio commented on PR #2245:
URL: https://github.com/apache/iggy/pull/2245#issuecomment-3388674031

   @chiradip missing verb ;)


-- 
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: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(sdk): Java SDK - :builder pattern for client #2202 [iggy]

Posted by "changhc (via GitHub)" <gi...@apache.org>.

changhc commented on PR #2245:
URL: https://github.com/apache/iggy/pull/2245#issuecomment-3375511409

   This is just my opinion, but I think it would be better to remove the issue number from the PR title. This keeps the title and the squashed commit headline short. Squashed commit headlines with two issue numbers like [this](https://github.com/apache/iggy/commit/0b77e33fc31123e919e6533742f1382086f5b0c6) are a bit confusing to me and I don't think having the source issue number in the commit headline brings much additional value when tracing commits.
   
   If the maintainers are fine with this, it's fine.


-- 
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: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org