Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nima WebClient configuration for timeouts and keepAlive #6971

Merged
merged 2 commits into from
Jun 21, 2023

Conversation

aserkes
Copy link
Contributor

@aserkes aserkes commented Jun 8, 2023

Fixes #6961
Fixes #6979

@aserkes aserkes self-assigned this Jun 8, 2023
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Jun 8, 2023
@aserkes aserkes requested a review from romain-grecourt June 8, 2023 16:30
Copy link
Contributor

@romain-grecourt romain-grecourt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is what can be done with the reactive WebClient in 3.x and 2.x :

WebClient client = WebClient.builder()
                            .connectTimeout(30, TimeUnit.SECONDS)
                            .readTimeout(10, TimeUnit.SECONDS)
                            .build();

WebClientResponse response = client.get()
                                   .connectTimeout(30, TimeUnit.SECONDS)
                                   .readTimeout(10, TimeUnit.SECONDS)
                                   .submit()
                                   .await();

Here is some Nima based code:

Http1Client client =
        Http1Client.builder()
                   .channelOptions(SocketOptions
                           .builder()
                           .connectTimeout(Duration.ofSeconds(30))
                           .readTimeout(Duration.ofSeconds(10))
                           .build())
                   //.connectTimeout(Duration.ofSeconds(30)) <-- shortcut
                   //.readTimeout(Duration.ofSeconds(10))    <-- shortcut
                   .build();
Http1ClientResponse response = client.get()
                                     //.connectTimeout(Duration.ofSeconds(30))  <-- if possible
                                     //.readTimeout(Duration.ofSeconds(10))     <-- if possible
                                     .request();

The commented out code above is what needs to be implemented.

@aserkes
Copy link
Contributor Author

aserkes commented Jun 9, 2023

If I understand it correctly, these timeouts related to the timeouts from SocketOptions. Therefore, to add the shortcuts and make these timeouts consistent with the timeouts from SocketOptions, as an option, I need to change SocketOptions and add a getter for socketOptions field in this class. Is it possible to change SocketOptions?

@romain-grecourt
Copy link
Contributor

Here are some rough steps

  • Add a new field to WebClient.Builder: SocketOptions.Builder channelOptionsBuilder.
  • In WebClient.Builder.build() if channelOptions is null, then assign a value using channelOptionsBuilder.build()
  • Update LoomClient constructor to remove the null check since channelOptionsBuilder is now never null
  • Add the following methods to WebBuilder.Builder:
public B readTimeout(Duration readTimeout) {
    this.channelOptionsBuilder.readTimeout(readTimeout);
    return (B) this;
}

public B connectTimeout(Duration connectTimeout) {
    this.channelOptionsBuilder.connectTimeout(connectTimeout);
    return (B) this;
}

@aserkes aserkes force-pushed the 6961_Nima_WebClient_timeouts branch 2 times, most recently from 938a686 to 5103d7f Compare June 15, 2023 07:18
@aserkes aserkes changed the title Nima WebClient Shortcuts for timeouts Nima WebClient configuration for timeouts and keepAlive Jun 15, 2023
@aserkes aserkes requested a review from romain-grecourt June 15, 2023 10:14
@aserkes aserkes requested a review from romain-grecourt June 19, 2023 14:52
aserkes and others added 2 commits June 20, 2023 20:53
Signed-off-by: aserkes <andrii.serkes@oracle.com>

add shortcuts for timeouts

Signed-off-by: aserkes <andrii.serkes@oracle.com>

clean code

Signed-off-by: aserkes <andrii.serkes@oracle.com>

WebClient config for keepAlive

Signed-off-by: aserkes <andrii.serkes@oracle.com>

fix checkstyle

Signed-off-by: aserkes <andrii.serkes@oracle.com>
…ild method.

This allows WebClient.Builder to run pre-build code to set the channel options from the default builder.

Slight re-org of WebClient.Builder:
 - move all package private methods at the bottom
 - add javadoc to protected methods

Update javadoc related to the default socket options builder.
@romain-grecourt romain-grecourt force-pushed the 6961_Nima_WebClient_timeouts branch from aa264c6 to 0ec2876 Compare June 21, 2023 03:59
@aserkes aserkes merged commit d1000c3 into helidon-io:main Jun 21, 2023
arjav-desai pushed a commit to arjav-desai/helidon that referenced this pull request Jun 22, 2023
)

* Nima WebClient Shortcuts for timeouts
* WebClient config for keepAlive
* Implement build() in WebClient.Builder and introduce an abstract doBuild method.
This allows WebClient.Builder to run pre-build code to set the channel options from the default builder.

Slight re-org of WebClient.Builder:
 - move all package private methods at the bottom
 - add javadoc to protected methods

Update javadoc related to the default socket options builder.

---------

Signed-off-by: aserkes <andrii.serkes@oracle.com>
Co-authored-by: Romain Grecourt <romain.grecourt@oracle.com>
arjav-desai pushed a commit to arjav-desai/helidon that referenced this pull request Jun 22, 2023
)

* Nima WebClient Shortcuts for timeouts
* WebClient config for keepAlive
* Implement build() in WebClient.Builder and introduce an abstract doBuild method.
This allows WebClient.Builder to run pre-build code to set the channel options from the default builder.

Slight re-org of WebClient.Builder:
 - move all package private methods at the bottom
 - add javadoc to protected methods

Update javadoc related to the default socket options builder.

---------

Signed-off-by: aserkes <andrii.serkes@oracle.com>
Co-authored-by: Romain Grecourt <romain.grecourt@oracle.com>
arjav-desai pushed a commit to arjav-desai/helidon that referenced this pull request Jun 23, 2023
)

* Nima WebClient Shortcuts for timeouts
* WebClient config for keepAlive
* Implement build() in WebClient.Builder and introduce an abstract doBuild method.
This allows WebClient.Builder to run pre-build code to set the channel options from the default builder.

Slight re-org of WebClient.Builder:
 - move all package private methods at the bottom
 - add javadoc to protected methods

Update javadoc related to the default socket options builder.

---------

Signed-off-by: aserkes <andrii.serkes@oracle.com>
Co-authored-by: Romain Grecourt <romain.grecourt@oracle.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants