From reviews-return-56950-apmail-mesos-reviews-archive=mesos.apache.org@mesos.apache.org Mon Mar 6 17:09:09 2017 Return-Path: X-Original-To: apmail-mesos-reviews-archive@minotaur.apache.org Delivered-To: apmail-mesos-reviews-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id BC909190CE for ; Mon, 6 Mar 2017 17:09:09 +0000 (UTC) Received: (qmail 74999 invoked by uid 500); 6 Mar 2017 17:09:09 -0000 Delivered-To: apmail-mesos-reviews-archive@mesos.apache.org Received: (qmail 74960 invoked by uid 500); 6 Mar 2017 17:09:09 -0000 Mailing-List: contact reviews-help@mesos.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: reviews@mesos.apache.org Delivered-To: mailing list reviews@mesos.apache.org Received: (qmail 74949 invoked by uid 99); 6 Mar 2017 17:09:09 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd3-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 06 Mar 2017 17:09:09 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd3-us-west.apache.org (ASF Mail Server at spamd3-us-west.apache.org) with ESMTP id F1468184928; Mon, 6 Mar 2017 17:09:08 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd3-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 1.203 X-Spam-Level: * X-Spam-Status: No, score=1.203 tagged_above=-999 required=6.31 tests=[DKIM_ADSP_CUSTOM_MED=0.001, HEADER_FROM_DIFFERENT_DOMAINS=0.001, HTML_MESSAGE=2, KAM_LAZY_DOMAIN_SECURITY=1, NML_ADSP_CUSTOM_MED=1.2, RP_MATCHES_RCVD=-2.999] autolearn=disabled Received: from mx1-lw-eu.apache.org ([10.40.0.8]) by localhost (spamd3-us-west.apache.org [10.40.0.10]) (amavisd-new, port 10024) with ESMTP id rGuy5MpiMqpP; Mon, 6 Mar 2017 17:09:07 +0000 (UTC) Received: from mailrelay1-us-west.apache.org (mailrelay1-us-west.apache.org [209.188.14.139]) by mx1-lw-eu.apache.org (ASF Mail Server at mx1-lw-eu.apache.org) with ESMTP id D3DFB5FC29; Mon, 6 Mar 2017 17:09:06 +0000 (UTC) Received: from reviews.apache.org (unknown [10.41.0.12]) by mailrelay1-us-west.apache.org (ASF Mail Server at mailrelay1-us-west.apache.org) with ESMTP id 1FCDFE0054; Mon, 6 Mar 2017 17:09:06 +0000 (UTC) Received: from reviews-vm2.apache.org (localhost [IPv6:::1]) by reviews.apache.org (ASF Mail Server at reviews-vm2.apache.org) with ESMTP id 1009EC40993; Mon, 6 Mar 2017 17:09:06 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============7840533305148986727==" MIME-Version: 1.0 Subject: Re: Review Request 57166: Updated role validation for hierarchical roles. From: Neil Conway To: Benjamin Bannier , Michael Park Cc: Neil Conway , Qian Zhang , mesos Date: Mon, 06 Mar 2017 17:09:06 -0000 Message-ID: <20170306170906.40992.78997@reviews-vm2.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: Neil Conway X-ReviewGroup: mesos X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/57166/ X-Sender: Neil Conway References: <20170302012113.10786.26009@reviews-vm2.apache.org> In-Reply-To: <20170302012113.10786.26009@reviews-vm2.apache.org> Reply-To: Neil Conway X-ReviewRequest-Repository: mesos --===============7840533305148986727== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On March 2, 2017, 1:21 a.m., Qian Zhang wrote: > > src/common/roles.cpp > > Line 71 (original), 71 (patched) > > > > > > Why can't role start with a slash? According to the design doc, it seems the role like `/eng/frontend` is valid. > > Neil Conway wrote: > Qian, this aspect of the hierarchical role design has changed since the design doc was initially proposed. The design doc has been partially updated -- I'll finish updating it shortly. The basic idea is that the syntax for hierarchical roles will just be an extension to the current syntax -- i.e., hierarchical role names will look like `eng/dev` and `eng/prod`, not `/eng/dev`. > > Qian Zhang wrote: > Thanks Neil for the clarification. But with this new design, the role name is more like a relative path rather than an absolute path, but I guess what we need is an absolute one, so when framework tries to register a role like `eng/dev`, we will always assume it is an absolute one (i.e., `eng` is root role), right? Role names do not begin with `/`, but they are effectively relative paths that are interpreted by starting from the root of the role tree. - Neil ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57166/#review167623 ----------------------------------------------------------- On March 2, 2017, 6:12 p.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57166/ > ----------------------------------------------------------- > > (Updated March 2, 2017, 6:12 p.m.) > > > Review request for mesos, Benjamin Bannier and Michael Park. > > > Repository: mesos > > > Description > ------- > > Role names can now contain forward slashes. Each component in a role > name must now be validated separately. > > > Diffs > ----- > > src/common/roles.cpp 31774a9b8f99f5efeed35b1c3e3486e05ca00f6a > src/tests/role_tests.cpp 77f3d46a544a51ba71476e2f0735bb32758dd9e1 > > > Diff: https://reviews.apache.org/r/57166/diff/2/ > > > Testing > ------- > > `make check` > > > Thanks, > > Neil Conway > > --===============7840533305148986727==--