> On April 23, 2018, 8:43 p.m., Benjamin Mahler wrote:
> > I'm not sure this approach is tenable, we may in the future have two
> > different `foo` fields with differing equality semantics.
> >
> > I was expecting that this patch would add an equality operator for specific
> > types we want to check. Do we need the `Message&` level equality operator?
>
> Kapil Arya wrote:
> Right now we don't have any differing equality semantics for any of the
> messages. There are four non-standard semantics are captured in the generic
> method. In future, when we need differring techniques for some bar, we can
> overload the `==` operator for that particular type. Does that sound
> reasonable?
>
> Since, MessageDifferencer recurses down to the fields without calling
> `==` itself, if we were to have two message Bar and Baz with with different
> equality semantics for `foo` fields in them , we'd can implement `==` for
> both Bar and Baz to reflect that. For example:
> ```
> operator==(const Bar& left, const Bar& right) {
> MessageDifferencer diff;
>
> // Bar-specific enhancements for Foo.
> diff.TreatAsSet(Foo::descriptor()->FindFieldByName("some_field"));
> ...
> return diff.compare(left, right);
> }
>
> operator==(const Baz& left, const Baz& right) {
> MessageDifferencer diff;
>
> // Baz-speficic enhancements for Foo.
> diff.TreatAsList(Foo::descriptor()->FindFieldByName("some_field"));
> ...
> return diff.compare(left, right);
> }
> ```
>
> However, if Bar and Baz were to have another common field `qux` with a
> non-standard comparator, we'd have to duplicate the qux-specific enhancement
> in both Bar and Baz comparators. This leads to some duplication, but that can
> be fixed with some clever maneuvering.
There is another possibility where we implement some generic
MesosMessageDifferencer like the one in the patch. Any implementations that
need to deviate from the standard then take a copy of MesosMessageDifferencer
and modify it as needed.
- Kapil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66746/#review201799
-----------------------------------------------------------
On April 20, 2018, 4:19 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66746/
> -----------------------------------------------------------
>
> (Updated April 20, 2018, 4:19 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The MessageDifferencer utility has been available since protobuf 3.0.0.
> When comparing most repeated fields, we treat them as sets so the order
> of elements doesn't matter. In some exceptional cases, we treat them as
> list so the order becomes important. Further, fields with default
> purposes are considered to be set for comparison purposes.
>
>
> Diffs
> -----
>
> include/mesos/type_utils.hpp 19ea81716496bcc0117a1b0ff157a0374f38bbfa
> include/mesos/v1/mesos.hpp fda3eb42061f820869a2d8da939fccadc4e5ddfb
> src/common/type_utils.cpp 33d63809b61a18e03ff745c88f024c71dd221ca2
> src/tests/master_allocator_tests.cpp
> e1aef8a9625a805e7ad2dfad37bfeedee82f160d
> src/v1/mesos.cpp 9b2df2dd798dff24a91a2604ab53c7fbb5ecfbcf
>
>
> Diff: https://reviews.apache.org/r/66746/diff/1/
>
>
> Testing
> -------
>
> `make check` currently fails with:
> DefaultExecutorCheckTest.CommandCheckSharesWorkDirWithTask
> DefaultExecutorCheckTest.MultipleTasksWithChecks
>
>
> Thanks,
>
> Kapil Arya
>
>
