You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Timothy Chen <tn...@apache.org> on 2015/07/03 23:30:01 UTC
Review Request 36173: Update attributes doc to reflect current
supported attributes types.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36173/
-----------------------------------------------------------
Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
Repository: mesos
Description
-------
Update attributes doc to reflect current supported attributes types.
Diffs
-----
docs/attributes-resources.md 0ae8b5908fe0b3a3499e6d813afbb328a13bdcde
Diff: https://reviews.apache.org/r/36173/diff/
Testing
-------
make
Thanks,
Timothy Chen
Re: Review Request 36173: Update attributes doc to reflect current
supported attributes types.
Posted by Timothy Chen <tn...@apache.org>.
> On July 6, 2015, 5:25 a.m., Adam B wrote:
> > docs/attributes-resources.md, line 31
> > <https://reviews.apache.org/r/36173/diff/1/?file=999015#file999015line31>
> >
> > No sets? Or are attributes implicitly sets?
I was suprised when I read the source code too, but we actually explicitly don't support sets for attributes yet.
> On July 6, 2015, 5:25 a.m., Adam B wrote:
> > docs/attributes-resources.md, line 39
> > <https://reviews.apache.org/r/36173/diff/1/?file=999015#file999015line39>
> >
> > 3 types? What about plain text?
Plain text is not a supported resource type. Not sure why but that's what the parse code looks like.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36173/#review90437
-----------------------------------------------------------
On July 3, 2015, 9:30 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36173/
> -----------------------------------------------------------
>
> (Updated July 3, 2015, 9:30 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Update attributes doc to reflect current supported attributes types.
>
>
> Diffs
> -----
>
> docs/attributes-resources.md 0ae8b5908fe0b3a3499e6d813afbb328a13bdcde
>
> Diff: https://reviews.apache.org/r/36173/diff/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 36173: Update attributes doc to reflect current
supported attributes types.
Posted by Timothy Chen <tn...@apache.org>.
> On July 6, 2015, 5:25 a.m., Adam B wrote:
> > docs/attributes-resources.md, line 11
> > <https://reviews.apache.org/r/36173/diff/1/?file=999015#file999015line11>
> >
> > s/either supported by Attributes or Resources/supported by Attributes and Resources/
> > And are sets a 'type' or just a collection of some other type (text)? Same with ranges, aren't they just a collection of scalars?
We do treat it as a different type since it has a different syntax of how it's defined.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36173/#review90437
-----------------------------------------------------------
On July 3, 2015, 9:30 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36173/
> -----------------------------------------------------------
>
> (Updated July 3, 2015, 9:30 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Update attributes doc to reflect current supported attributes types.
>
>
> Diffs
> -----
>
> docs/attributes-resources.md 0ae8b5908fe0b3a3499e6d813afbb328a13bdcde
>
> Diff: https://reviews.apache.org/r/36173/diff/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 36173: Update attributes doc to reflect current
supported attributes types.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36173/#review90437
-----------------------------------------------------------
Ship it!
Seems like scalar and text are basic types, and then ranges and sets are compound types. Sounds weird to talk of 4 types that apply to either, and then only name 3 for each.
docs/attributes-resources.md (line 11)
<https://reviews.apache.org/r/36173/#comment143482>
s/either supported by Attributes or Resources/supported by Attributes and Resources/
And are sets a 'type' or just a collection of some other type (text)? Same with ranges, aren't they just a collection of scalars?
docs/attributes-resources.md (lines 25 - 27)
<https://reviews.apache.org/r/36173/#comment143485>
s/labelString/text/g?
docs/attributes-resources.md (line 31)
<https://reviews.apache.org/r/36173/#comment143483>
No sets? Or are attributes implicitly sets?
docs/attributes-resources.md (line 39)
<https://reviews.apache.org/r/36173/#comment143484>
3 types? What about plain text?
- Adam B
On July 3, 2015, 2:30 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36173/
> -----------------------------------------------------------
>
> (Updated July 3, 2015, 2:30 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Update attributes doc to reflect current supported attributes types.
>
>
> Diffs
> -----
>
> docs/attributes-resources.md 0ae8b5908fe0b3a3499e6d813afbb328a13bdcde
>
> Diff: https://reviews.apache.org/r/36173/diff/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 36173: Update attributes doc to reflect current
supported attributes types.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36173/#review90445
-----------------------------------------------------------
docs/attributes-resources.md (line 35)
<https://reviews.apache.org/r/36173/#comment143491>
Doesn't `( scalar | range | text | "," )+` look like a set that doesn't wrap itself in `{}` braces?
It doesn't look like Attributes::parse handles the "," case, so let's remove that (and the `()+`) from the documentation.
- Adam B
On July 3, 2015, 2:30 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36173/
> -----------------------------------------------------------
>
> (Updated July 3, 2015, 2:30 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Update attributes doc to reflect current supported attributes types.
>
>
> Diffs
> -----
>
> docs/attributes-resources.md 0ae8b5908fe0b3a3499e6d813afbb328a13bdcde
>
> Diff: https://reviews.apache.org/r/36173/diff/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 36173: Update attributes doc to reflect current
supported attributes types.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36173/#review90389
-----------------------------------------------------------
Patch looks great!
Reviews applied: [36173]
All tests passed.
- Mesos ReviewBot
On July 3, 2015, 9:30 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36173/
> -----------------------------------------------------------
>
> (Updated July 3, 2015, 9:30 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Update attributes doc to reflect current supported attributes types.
>
>
> Diffs
> -----
>
> docs/attributes-resources.md 0ae8b5908fe0b3a3499e6d813afbb328a13bdcde
>
> Diff: https://reviews.apache.org/r/36173/diff/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Timothy Chen
>
>