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

Implement Http.closeAndConfigure, start migration path for 0.13.x #156

Merged
merged 5 commits into from Jun 3, 2017

Conversation

farmdawgnation
Copy link
Member

@farmdawgnation farmdawgnation commented May 27, 2017

This addresses #53 and other issues in such a way that does not change
underlying behavior in the 0.12.x series. Now, people using Dispatch
0.12.x will get a warning if they attempt to use the configure method
directly.

This has the benefit of not changing the underlying behavior in a minor
release. We will warn using a deprecation warning that this method might
be unsafe, but will permit callers to continue using the Http instance
after invoking it if their application depends on that behavior.

We add a closeAndConfigure method that's the intended "safe" method to
invoke in most cases.

Closes #53
Closes #59
Closes #114

This also makes some changes to help folks start migrating to Dispatch 0.13.x, where we'll start disallowing the use of the Http singleton directly to avoid situations where two characters can accidentally cause additional resource allocation. For more information, see #157.

This addresses #53 and other issues in such a way that does not change
underlying behavior in the 0.12.x series. Now, people using Dispatch
0.12.x will get a warning if they attempt to use the configure method
directly.

This has the benefit of not changing the underlying behavior in a minor
release. We will warn using a deprecation warning that this method might
be unsafe, but will permit callers to continue using the Http instance
after invoking it if their application depends on that behavior.

We add a closeAndConfigure method that's the intended "safe" method to
invoke in most cases.
farmdawgnation added a commit that referenced this pull request May 27, 2017
This commit forward-ports the API changes from #156 into
the 0.13.x release series with a few important changes to boot.

The first, and perhaps biggest, change is that the Http singleton is no
longer an Http executor in it own right. This will no doubt cause some
code breakage that has to be addressed as folks upgrade to 0.13.x. In
order to minimize that code breakage, Http.default provides a default
executor that is always usable for just "doing some HTTP" without
further customization.

This will make explicit the fact that you're invoking a real Thing™ that
has resources that will need to be cleaned up when you're done using
them. This was something that could be ambiguous in earlier versions of
Dispatch because if you happened to invoke `Http()` instead of `Http`
you'd get a new resource pool each time and quickly create problems for
yourself. Two characters should not make that much of a difference in
behavior.

Furthermore, we no longer provide the default builder to the Http case
class by default. It's accessable at Http.defaultClientBuilder if you
wish to use it.

This also adds Http.withConfiguration to facilitate configuring custom
Http instances.
After implementing the 0.13.x version of this fix, it became clear that
there were some additional thoughts that needed to be added here.
This commit adds deprecation warnings on the direct use of the Http
singleton and adds an implementation of Http.default so that there is an
easy migration path from Dispatch 0.12.x to Dispatch 0.13.x
This ensures that we have one place to add any additional shutdown logic
in the future.
@farmdawgnation farmdawgnation changed the title Implement Http.closeAndConfigure Implement Http.closeAndConfigure, start migration path for 0.13.x May 27, 2017
@farmdawgnation farmdawgnation merged commit 59fa469 into 0.12.x Jun 3, 2017
@farmdawgnation farmdawgnation deleted the close-client-before-configure branch June 3, 2017 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant