feat(encoding)!: 🧁 encoding traits do not consume encoders #240
Conversation
cratelyn
force-pushed
the
encode-mut-ref
branch
from
ca76c67 to
7d99951
Compare
|
i've refrained from bumping the |
cratelyn
force-pushed
the
encode-mut-ref
branch
from
7d99951 to
8175d61
Compare
|
i've also updated the benches, examples, and doctests so this should be ready for review now. 🔨 |
cratelyn
force-pushed
the
encode-mut-ref
branch
from
8175d61 to
99cc7c4
Compare
see prometheus#135. this adjusts the parameter of `encode()` so that it only mutably borrows the encoder. Signed-off-by: katelyn martin <[email protected]>
see prometheus#135. this adjusts the parameter of `encode()` so that it only mutably borrows the encoder. Signed-off-by: katelyn martin <[email protected]>
see prometheus#135. this adjusts the parameter of `encode()` so that it only mutably borrows the encoder. Signed-off-by: katelyn martin <[email protected]>
see prometheus#135. this adjusts the parameter of `encode()` so that it only mutably borrows the encoder. Signed-off-by: katelyn martin <[email protected]>
…eEncoder` see prometheus#135. this adjusts the parameter of `encode()` so that it only mutably borrows the encoder. Signed-off-by: katelyn martin <[email protected]>
cratelyn
force-pushed
the
encode-mut-ref
branch
from
99cc7c4 to
8833b0b
Compare
|
this work conflicts with the cosmetic changes that were made in #237. i'm happy to rebase this branch and address those conflicts, but i'd like to hold off on doing so until i have a signal that we'd be disposed to merge these changes. please ping me if, or when, that work would be desired. |
|
Thinking about this some more, I think the choice of taking ownership was deliberate when I designed these builder patterns. Can you come up with an example that is not supported by the status quo? |
|
Yes, It's not possible to pair two arbitrary labelsets, for example. We end up with this extra trait that we have to implement on all of our EncodeLabelSet impls Which is needed to implement something like: Additionally, as I noted in the initial issue, this would support |
|
@olix0r thanks for the quick response. Sorry for the delay @cratelyn and @olix0r. I have not considered the use-case above before, namely to combine two label sets. I think it is a valid use-case. Instead of changing all signatures, I would prefer only doing it for those, where we have a valid use-case, e.g. the above use-case with Would either of you be willing to provide a pull request for the change to |
this commit alters the signature of the `EncodeLabelSet::encode()` trait method, such that it now accepts a mutable reference to its encoder. this is related to prometheus#135, and is a second proposal following previous work in prometheus#240. this change permits distinct label sets to be composed together, now that the label set encoder is not consumed. a new implementation for tuples `(A, B)` is provided. this commit includes a test case showing that a metric family can compose two label sets together, and that such a family can successfully be digested by the python client library. `derive-encode` is altered to generate code matching this new trait signature, and has been bumped to version 0.5.0 as a result of this breaking change in the `prometheus-client` library. Signed-off-by: katelyn martin <[email protected]>
this commit alters the signature of the `EncodeLabelSet::encode()` trait method, such that it now accepts a mutable reference to its encoder. this is related to prometheus#135, and is a second proposal following previous work in prometheus#240. this change permits distinct label sets to be composed together, now that the label set encoder is not consumed. a new implementation for tuples `(A, B)` is provided. this commit includes a test case showing that a metric family can compose two label sets together, and that such a family can successfully be digested by the python client library. `derive-encode` is altered to generate code matching this new trait signature, and has been bumped to version 0.5.0 as a result of this breaking change in the `prometheus-client` library. Signed-off-by: katelyn martin <[email protected]>
this commit alters the signature of the `EncodeLabelSet::encode()` trait method, such that it now accepts a mutable reference to its encoder. this is related to prometheus#135, and is a second proposal following previous work in prometheus#240. this change permits distinct label sets to be composed together, now that the label set encoder is not consumed. a new implementation for tuples `(A, B)` is provided. this commit includes a test case showing that a metric family can compose two label sets together, and that such a family can successfully be digested by the python client library. `derive-encode` is altered to generate code matching this new trait signature, and has been bumped to version 0.5.0 as a result of this breaking change in the `prometheus-client` library. Signed-off-by: katelyn martin <[email protected]>
|
succeeded by #257. |
this commit alters the signature of the `EncodeLabelSet::encode()` trait method, such that it now accepts a mutable reference to its encoder. this is related to prometheus#135, and is a second proposal following previous work in prometheus#240. this change permits distinct label sets to be composed together, now that the label set encoder is not consumed. a new implementation for tuples `(A, B)` is provided. this commit includes a test case showing that a metric family can compose two label sets together, and that such a family can successfully be digested by the python client library. `derive-encode` is altered to generate code matching this new trait signature, and has been bumped to version 0.5.0 as a result of this breaking change in the `prometheus-client` library. Signed-off-by: katelyn martin <[email protected]>
fixes #135.
this branch updates parameter types for different encoding traits:
EncodeLabel,EncodeLabelSet,EncodeMetric,Collector, andEncodeExemplarValue. rather than taking ownership of their respective encoder parameters, these now accept a&mutmutable reference.this allows for consumers of this API to e.g. compose label sets, loosens restrictions on field order in
#[derive(EncodeLabelSet)]structs, and provides a consistent signature type by following the pattern already set byEncodeLabelKey,EncodeLabelValue,EncodeGaugeValue, and other existing encoding traits.