Noteworthy changes:
- All constructors now follow the same pattern: `new` uses OS entropy,
`from_rng` takes a `R: Rng` and seeds the randomness from there.
`from_rng` is fallible, since randomness generators can be fallible.
- `BalanceLayer` was renamed to `MakeBalanceLayer`, since it is not
_really_ a `BalanceLayer`. The name of `BalanceMake` was also
"normalized" to `MakeBalance`.
Another observation: the `Debug` bound on `Load::Metric` in
`p2c::Balance`, while not particularly onerous, generates really
confusing errors if you forget it include it. And crucially, the error
never points at `Debug` (should we file a compiler issue?), so I pretty
much had to guess my way to that being wrong in the doc example.
It would probably be useful to add a documentation example to
`MakeBalanceLayer` or `MakeBalance` (I suspect just one of them is fine,
since they're basically the same). Since I've never used it, and find it
hard to think of uses for it, it might be good if someone with more
experience with it wrote one.
This was a mostly mechanical change. I think in at least one place it
results in a `'static` bound being added, but the next tower release
will be breaking anyway, so that's okay.
I think it helps to also document the alias at the top to (eventually)
explain how people can interact with the error they get back to discover
the "deeper cause".
## Motivation
Commit #330 introduced a regression when porting `tower-util::Oneshot`
from `futures` 0.1 to `std::future`. The *intended* behavior is that a
oneshot future should repeatedly call `poll_ready` on the oneshotted
service until it is ready, and then call the service and drive the
returned future. However, #330 inadvertently changed the oneshot future
to poll the service _once_, call it if it is ready, and then drop it,
regardless of its readiness.
In the #330 version of oneshot, an `Option` is used to store the
request while waiting for the service to become ready, so that it can be
`take`n and moved into the service's `call`. However, the `Option`
contains both the request _and_ the service itself, and is taken the
first time the service is polled. `futures::ready!` is then used when
polling the service, so the method returns immediate if it is not ready.
This means that the service itself (and the request), which were taken
out of the `Option`, will be dropped, and if the oneshot future is
polled again, it will panic.
## Solution
This commit changes the `Oneshot` future so that only the request lives
in the `Option`, and it is only taken when the service is called, rather
than every time it is polled. This fixes the bug.
I've also added a test for this which fails against master, but passes
after this change.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
This also renames the `Instrument` trait, and related types, to better
reflect what they do. Specifically, the trait is now called
`TrackCompletion`, and `NoInstrument` is called `CompleteOnResponse`.
Also brings back balance example and makes it compile.
* Change Discover to be a sealed trait
`Discover` was _really_ just a `TryStream<Item = Change>`, so this
change makes that much clearer. Specifically, users are intended to use
`Discover` only in bounds, whereas implementors should implement
`Stream` with the appropriate `Item` type. `Discover` then comes with a
blanket implementation for anything that implements `TryStream`
appropriately. This obviates the need for the `discover::stream` module.
* util: Add ReadyAnd to do what Ready should do
`ServiceExt::ready` says that it produces "A future yielding the service
when it is ready to accept a request." This is not true; it does _not_
yield the service when it is ready, it yields unit. This makes it
impossible to chain service ready with service call, which is sad.
This PR adds `ready_and`, which does what `ready` promised. It also
deprecates `ready` with the intention that we remove `ready` in a future
version, and make the strictly more general `ready_and` take its place.
We can't do it now since it's not a backwards-compatible change even
though it _probably_ wouldn't break any code.
The PR also updates the docs so that they reflect the observed behavior.
This is of particular importance since the `log` feature of `tracing`
(currently) fails to compile if the `tracing` dependency is renamed.
Without a way to disable it in `tower`, any package that both depends on
`tower` **and** renames `tracing` in its dependencies is doomed.
This makes all tower subcrates have the following lints as warn (rather
than allow): `missing_docs`, `rust_2018_idioms`, `unreachable_pub`, and
`missing_debug_implementations`. In addition, it consistently applies
`deny(warning)` *only* under CI so that deprecations and macro changes in minor
version bumps in dependencies will never cause `tower` crates to stop
compiling, and so that tests can be run even if not all warnings have been
dealt with. See also https://github.com/rust-unofficial/patterns/blob/master/anti_patterns/deny-warnings.md
Note that `tower-reconnect` has the `missing_docs` lint disabled for now
since it contained _no_ documentation previously. Also note that this
patch does not add documentation to the various `new` methods, as they
are considered self-explanatory. They are instead marked as
`#[allow(missing_docs)]`.
Rather than consuming `self` and returning `(Self, _)`. This did mean
that a few crates that depended on `Ready` to own the `Service` and
provide it once it was ready had to change to call `poll_ready`
directly. Which in turn meant adding in some PhantomData<Request> so
that the impl blocks wouldn't be under-constrainted. Take, for example:
```
impl<K, S: Service<Req>, Req> Future for UnreadyService<K, S>
```
would fail to compile with
```
error[E0207]: the type parameter `Req` is not constrained by the impl trait, self type, or predicates
```
When using a `ServiceBuilder`, it's not possible to obtain the
underlying `Layer` implementation.
Adding a `ServiceBuilder::into_inner` allows callers to retrieve this
field instead of only being able to build a `Service`.
Taiki Endo
Bruce Guenter
Jon Gjengset
Eliza Weisman
Lucio Franco
Akshay Narayan
Sean McArthur
Juan Alvarez
Oliver Gould
Carl Lerche