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

Manage Apache HTTP and Jersey HTTP clients #1061

Merged
merged 2 commits into from Jun 2, 2015

Conversation

Projects
None yet
3 participants
@arteam
Member

arteam commented May 20, 2015

In the documentation we say that we create managed instances of Apache HTTP client. But in the fact, it's not true.

To follow this contract, we should tie the lifecyle of the client to the lifecycle of the server, if the user specified the environment in the builder. The rationale is the same for Jersey HTTP client.

If the user has already created own managed object for the client - it's not an issue, because the close method of the HTTP client is idempotent.

@jplock

This comment has been minimized.

Member

jplock commented May 20, 2015

LGTM, I'll see if anyone else would like to review as well

@jplock jplock added this to the 0.9.0 milestone May 20, 2015

@carlo-rtr

This comment has been minimized.

Member

carlo-rtr commented May 26, 2015

Looks good. One minor nitpick, should we add a test in JerseyClientBuilderTest?

arteam added some commits May 20, 2015

Create managed instances of Apache HTTP Client
In the documentation we say that we create managed instances of
Apache HTTP client. But in the fact, it's not true.

We should tie the lifecyle of the HTTP client to the lifecycle of
the server, if the user specified the environment in the builder.

If the user has already created own managed object - it's not:w
an issue, because the close method of the HTTP client is idempotent.
Create managed instances of Jersey HTTP Client
The rationale is the same as for Apache HTTP Client. We say in the
documentation that an instance of the client is managed by the server.
Therefore, we should follow this contract, if the user supplied
the environment to us.
@arteam

This comment has been minimized.

Member

arteam commented Jun 1, 2015

I've added a check in DropwizardApacheConnectorTest#tearDown method.
We can't create a mock Jersey client, so we check that the client is closed in a real environment.

jplock added a commit that referenced this pull request Jun 2, 2015

Merge pull request #1061 from arteam/manage_clients
Manage Apache HTTP and Jersey HTTP clients

@jplock jplock merged commit 398b9c9 into dropwizard:master Jun 2, 2015

@arteam arteam deleted the arteam:manage_clients branch Jan 24, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment