Re: Review Request 37996: Added InheritanceTree a tree based container where children nodes inherit the values associated with their parent.

2015-10-13 Thread Alexander Rojas


> On Oct. 6, 2015, 2:39 p.m., Bernd Mathiske wrote:
> > Ship It!
> 
> Ben Mahler wrote:
> I don't think we should introduce this into stout in its current form. I 
> realize you're planning to use this for authentication stuff, but looking at 
> this on its own, it seems like a confusing abstraction. Why would we couple 
> the notion of a Tree with the semantics around properties and property 
> inheritance?
> 
> Till Toenshoff wrote:
> This code is being used in libprocess. So the options are libprocess or 
> stout for introducing it. I believe it would be a better fit for stout than 
> for libprocess as it is a data structure implementation that has no threading 
> or process specifics. Given that it already is in a reusable state, I think 
> that we should go as proposed by this RR.
> 
> Bernd Mathiske wrote:
> @bmahler, what would you rather have us do? This structure is quite close 
> to Boost's PropertyTree 
> (http://www.boost.org/doc/libs/1_59_0/doc/html/property_tree.html), and we 
> would have loved to use that one. But it does not feature inheritance, which 
> seems a natural, essential thing to have in at least some trees.
> 
> Ben Mahler wrote:
> Hm... the "inheritance" stuff that you guys need seems to just be a walk 
> of the tree from the root to the leaf you're interested in, collecting 
> properties of nodes along the way. Any reason you can't layer that 
> functionality on top or, even better, on the side via a free standing 
> function so that you don't need to introduce a new data structure?
> 
> Jie Yu wrote:
> +1 if we can compose/inherit from boost property tree rather than 
> implementing our own.
> 
> Alexander Rojas wrote:
> Property tree as of the moment is not distributed with Mesos. However, 
> there is a reason why it wasn't added, and it is that it will require a 
> double parsing of the path (one when asking the ptree, the other manually to 
> verify every subpath).
> 
> Ben Mahler wrote:
> In other words, you were concerned about performance?

That and that pulling property tree from boost will include the whole boost 
spirit. Still, I will withdrawn this patch and follow your suggestion of puting 
this structure only where it will be used.


- Alexander


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37996/#review101639
---


On Oct. 7, 2015, 11:54 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37996/
> ---
> 
> (Updated Oct. 7, 2015, 11:54 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the `InheritanceTree` class which allows to create a tree where 
> nodes can be tag with some property. This property is then inherited by 
> children nodes.
> 
> Two behaviors are implemented, overriding, i.e. Adding a property to the 
> child node of another node with a property already will result in the 
> ancestor property being lost. The second behavior, accumulating, takes a 
> function and accumulates
> properties of all ancestors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 76e1674e08bbe65a4fdf86731823a61f231d6d12 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
>   3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 37996: Added InheritanceTree a tree based container where children nodes inherit the values associated with their parent.

2015-10-12 Thread Ben Mahler


> On Oct. 6, 2015, 12:39 p.m., Bernd Mathiske wrote:
> > Ship It!
> 
> Ben Mahler wrote:
> I don't think we should introduce this into stout in its current form. I 
> realize you're planning to use this for authentication stuff, but looking at 
> this on its own, it seems like a confusing abstraction. Why would we couple 
> the notion of a Tree with the semantics around properties and property 
> inheritance?
> 
> Till Toenshoff wrote:
> This code is being used in libprocess. So the options are libprocess or 
> stout for introducing it. I believe it would be a better fit for stout than 
> for libprocess as it is a data structure implementation that has no threading 
> or process specifics. Given that it already is in a reusable state, I think 
> that we should go as proposed by this RR.
> 
> Bernd Mathiske wrote:
> @bmahler, what would you rather have us do? This structure is quite close 
> to Boost's PropertyTree 
> (http://www.boost.org/doc/libs/1_59_0/doc/html/property_tree.html), and we 
> would have loved to use that one. But it does not feature inheritance, which 
> seems a natural, essential thing to have in at least some trees.
> 
> Ben Mahler wrote:
> Hm... the "inheritance" stuff that you guys need seems to just be a walk 
> of the tree from the root to the leaf you're interested in, collecting 
> properties of nodes along the way. Any reason you can't layer that 
> functionality on top or, even better, on the side via a free standing 
> function so that you don't need to introduce a new data structure?
> 
> Jie Yu wrote:
> +1 if we can compose/inherit from boost property tree rather than 
> implementing our own.
> 
> Alexander Rojas wrote:
> Property tree as of the moment is not distributed with Mesos. However, 
> there is a reason why it wasn't added, and it is that it will require a 
> double parsing of the path (one when asking the ptree, the other manually to 
> verify every subpath).

In other words, you were concerned about performance?


- Ben


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37996/#review101639
---


On Oct. 7, 2015, 9:54 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37996/
> ---
> 
> (Updated Oct. 7, 2015, 9:54 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the `InheritanceTree` class which allows to create a tree where 
> nodes can be tag with some property. This property is then inherited by 
> children nodes.
> 
> Two behaviors are implemented, overriding, i.e. Adding a property to the 
> child node of another node with a property already will result in the 
> ancestor property being lost. The second behavior, accumulating, takes a 
> function and accumulates
> properties of all ancestors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 76e1674e08bbe65a4fdf86731823a61f231d6d12 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
>   3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 37996: Added InheritanceTree a tree based container where children nodes inherit the values associated with their parent.

2015-10-12 Thread Alexander Rojas


> On Oct. 6, 2015, 2:39 p.m., Bernd Mathiske wrote:
> > Ship It!
> 
> Ben Mahler wrote:
> I don't think we should introduce this into stout in its current form. I 
> realize you're planning to use this for authentication stuff, but looking at 
> this on its own, it seems like a confusing abstraction. Why would we couple 
> the notion of a Tree with the semantics around properties and property 
> inheritance?
> 
> Till Toenshoff wrote:
> This code is being used in libprocess. So the options are libprocess or 
> stout for introducing it. I believe it would be a better fit for stout than 
> for libprocess as it is a data structure implementation that has no threading 
> or process specifics. Given that it already is in a reusable state, I think 
> that we should go as proposed by this RR.
> 
> Bernd Mathiske wrote:
> @bmahler, what would you rather have us do? This structure is quite close 
> to Boost's PropertyTree 
> (http://www.boost.org/doc/libs/1_59_0/doc/html/property_tree.html), and we 
> would have loved to use that one. But it does not feature inheritance, which 
> seems a natural, essential thing to have in at least some trees.
> 
> Ben Mahler wrote:
> Hm... the "inheritance" stuff that you guys need seems to just be a walk 
> of the tree from the root to the leaf you're interested in, collecting 
> properties of nodes along the way. Any reason you can't layer that 
> functionality on top or, even better, on the side via a free standing 
> function so that you don't need to introduce a new data structure?
> 
> Jie Yu wrote:
> +1 if we can compose/inherit from boost property tree rather than 
> implementing our own.

Property tree as of the moment is not distributed with Mesos. However, there is 
a reason why it wasn't added, and it is that it will require a double parsing 
of the path (one when asking the ptree, the other manually to verify every 
subpath).


- Alexander


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37996/#review101639
---


On Oct. 7, 2015, 11:54 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37996/
> ---
> 
> (Updated Oct. 7, 2015, 11:54 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the `InheritanceTree` class which allows to create a tree where 
> nodes can be tag with some property. This property is then inherited by 
> children nodes.
> 
> Two behaviors are implemented, overriding, i.e. Adding a property to the 
> child node of another node with a property already will result in the 
> ancestor property being lost. The second behavior, accumulating, takes a 
> function and accumulates
> properties of all ancestors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 76e1674e08bbe65a4fdf86731823a61f231d6d12 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
>   3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 37996: Added InheritanceTree a tree based container where children nodes inherit the values associated with their parent.

2015-10-08 Thread Jie Yu


> On Oct. 6, 2015, 12:39 p.m., Bernd Mathiske wrote:
> > Ship It!
> 
> Ben Mahler wrote:
> I don't think we should introduce this into stout in its current form. I 
> realize you're planning to use this for authentication stuff, but looking at 
> this on its own, it seems like a confusing abstraction. Why would we couple 
> the notion of a Tree with the semantics around properties and property 
> inheritance?
> 
> Till Toenshoff wrote:
> This code is being used in libprocess. So the options are libprocess or 
> stout for introducing it. I believe it would be a better fit for stout than 
> for libprocess as it is a data structure implementation that has no threading 
> or process specifics. Given that it already is in a reusable state, I think 
> that we should go as proposed by this RR.
> 
> Bernd Mathiske wrote:
> @bmahler, what would you rather have us do? This structure is quite close 
> to Boost's PropertyTree 
> (http://www.boost.org/doc/libs/1_59_0/doc/html/property_tree.html), and we 
> would have loved to use that one. But it does not feature inheritance, which 
> seems a natural, essential thing to have in at least some trees.
> 
> Ben Mahler wrote:
> Hm... the "inheritance" stuff that you guys need seems to just be a walk 
> of the tree from the root to the leaf you're interested in, collecting 
> properties of nodes along the way. Any reason you can't layer that 
> functionality on top or, even better, on the side via a free standing 
> function so that you don't need to introduce a new data structure?

+1 if we can compose/inherit from boost property tree rather than implementing 
our own.


- Jie


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37996/#review101639
---


On Oct. 7, 2015, 9:54 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37996/
> ---
> 
> (Updated Oct. 7, 2015, 9:54 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the `InheritanceTree` class which allows to create a tree where 
> nodes can be tag with some property. This property is then inherited by 
> children nodes.
> 
> Two behaviors are implemented, overriding, i.e. Adding a property to the 
> child node of another node with a property already will result in the 
> ancestor property being lost. The second behavior, accumulating, takes a 
> function and accumulates
> properties of all ancestors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 76e1674e08bbe65a4fdf86731823a61f231d6d12 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
>   3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 37996: Added InheritanceTree a tree based container where children nodes inherit the values associated with their parent.

2015-10-07 Thread Ben Mahler


> On Oct. 6, 2015, 12:39 p.m., Bernd Mathiske wrote:
> > Ship It!
> 
> Ben Mahler wrote:
> I don't think we should introduce this into stout in its current form. I 
> realize you're planning to use this for authentication stuff, but looking at 
> this on its own, it seems like a confusing abstraction. Why would we couple 
> the notion of a Tree with the semantics around properties and property 
> inheritance?
> 
> Till Toenshoff wrote:
> This code is being used in libprocess. So the options are libprocess or 
> stout for introducing it. I believe it would be a better fit for stout than 
> for libprocess as it is a data structure implementation that has no threading 
> or process specifics. Given that it already is in a reusable state, I think 
> that we should go as proposed by this RR.
> 
> Bernd Mathiske wrote:
> @bmahler, what would you rather have us do? This structure is quite close 
> to Boost's PropertyTree 
> (http://www.boost.org/doc/libs/1_59_0/doc/html/property_tree.html), and we 
> would have loved to use that one. But it does not feature inheritance, which 
> seems a natural, essential thing to have in at least some trees.

Hm... the "inheritance" stuff that you guys need seems to just be a walk of the 
tree from the root to the leaf you're interested in, collecting properties of 
nodes along the way. Any reason you can't layer that functionality on top or, 
even better, on the side via a free standing function so that you don't need to 
introduce a new data structure?


- Ben


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37996/#review101639
---


On Oct. 7, 2015, 9:54 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37996/
> ---
> 
> (Updated Oct. 7, 2015, 9:54 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the `InheritanceTree` class which allows to create a tree where 
> nodes can be tag with some property. This property is then inherited by 
> children nodes.
> 
> Two behaviors are implemented, overriding, i.e. Adding a property to the 
> child node of another node with a property already will result in the 
> ancestor property being lost. The second behavior, accumulating, takes a 
> function and accumulates
> properties of all ancestors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 76e1674e08bbe65a4fdf86731823a61f231d6d12 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
>   3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 37996: Added InheritanceTree a tree based container where children nodes inherit the values associated with their parent.

2015-10-07 Thread Alexander Rojas

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37996/
---

(Updated Oct. 7, 2015, 11:54 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Update summary.


Summary (updated)
-

Added InheritanceTree a tree based container where children nodes inherit the 
values associated with their parent.


Bugs: MESOS-3231
https://issues.apache.org/jira/browse/MESOS-3231


Repository: mesos


Description
---

Introduces the `InheritanceTree` class which allows to create a tree where 
nodes can be tag with some property. This property is then inherited by 
children nodes.

Two behaviors are implemented, overriding, i.e. Adding a property to the child 
node of another node with a property already will result in the ancestor 
property being lost. The second behavior, accumulating, takes a function and 
accumulates
properties of all ancestors.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/Makefile.am 
76e1674e08bbe65a4fdf86731823a61f231d6d12 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
94292f8a46ec31bbaf6e52f48109322bbe123f70 
  3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp 
PRE-CREATION 

Diff: https://reviews.apache.org/r/37996/diff/


Testing
---

make check


Thanks,

Alexander Rojas