Mailing-List: contact reviews-help@mesos.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@mesos.apache.org
Content-Type: multipart/alternative;
 boundary="===============8380169846220953313=="
MIME-Version: 1.0
Subject: Re: Review Request 56645: Added agent capabilities to `/state`(v0)
 endpoint of agent.
From: Jay Guo <guojiannan1101@gmail.com>
To: Benjamin Bannier <benjamin.bannier@mesosphere.io>,
 Benjamin Mahler <bmahler@apache.org>, Guangya Liu <gyliu@apache.org>,
 Michael Park <mpark@apache.org>
Cc: Jay Guo <guojiannan1101@gmail.com>, mesos <reviews@mesos.apache.org>
Date: Wed, 15 Feb 2017 02:22:21 -0000
Message-ID: <20170215022221.13057.80513@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: Jay Guo <noreply@reviews.apache.org>
References: <20170214095724.31018.63727@reviews.apache.org>
In-Reply-To: <20170214095724.31018.63727@reviews.apache.org>
Reply-To: Jay Guo <guojiannan1101@gmail.com>
archived-at: Wed, 15 Feb 2017 02:22:24 -0000

--===============8380169846220953313==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> On Feb. 14, 2017, 5:57 p.m., Benjamin Bannier wrote:
> > src/slave/http.cpp, lines 1246-1247
> > <https://reviews.apache.org/r/56645/diff/1/?file=1633008#file1633008line1246>
> >
> >     I see we have a type `SlaveInfo::Capability` defined, but don't store any values there. Shouldn't we have a repeated field there (similar to what we currently have for e.g., agent attributes, instead of synthetizing something on the spot here?
> 
> Benjamin Bannier wrote:
>     Per our offline discussion, not storing this information in `SlaveInfo` is deliberate and follows the pattern already in place for version which is also communicated via `RegisterSlaveMessage`s instead of configured in `SlaveInfo`. As hardcoding the same thing in two different places doesn't seem too resilent, do you think it would make sense to at least introduce a constant for agent capabilities that we could reference in both `slave/http`, and agent registration code (the version is already taken from a constant `MESOS_VERSION`)?

What do you mean by _two different places_? I think we only store them in slave.capabilities per previous commit, no?


- Jay


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


On Feb. 14, 2017, 6:19 p.m., Jay Guo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56645/
> -----------------------------------------------------------
> 
> (Updated Feb. 14, 2017, 6:19 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and Michael Park.
> 
> 
> Bugs: MESOS-6902
>     https://issues.apache.org/jira/browse/MESOS-6902
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added agent capabilities to `/state`(v0) endpoint of agent.
> 
> 
> Diffs
> -----
> 
>   src/common/http.hpp 3d5ab59ddc4dce4d791c1b439f5e1459d1a724a4 
>   src/common/http.cpp abfbf7248beb2d4068be06b7f5f829d7617f943e 
>   src/master/http.cpp caa1fabb7a709d993ff711cc7803dd05000fa987 
>   src/slave/http.cpp ccd489d700a605dc41037e00dbd6a7cb3b6edcc6 
>   src/tests/slave_tests.cpp b6f76824c20c842d8f1d8afb1f9b81668b6741da 
> 
> Diff: https://reviews.apache.org/r/56645/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>


--===============8380169846220953313==--