Return-Path: X-Original-To: apmail-mesos-dev-archive@www.apache.org Delivered-To: apmail-mesos-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 6CD6D10962 for ; Thu, 20 Nov 2014 00:58:04 +0000 (UTC) Received: (qmail 81898 invoked by uid 500); 20 Nov 2014 00:58:04 -0000 Delivered-To: apmail-mesos-dev-archive@mesos.apache.org Received: (qmail 81835 invoked by uid 500); 20 Nov 2014 00:58:04 -0000 Mailing-List: contact dev-help@mesos.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@mesos.apache.org Delivered-To: mailing list dev@mesos.apache.org Received: (qmail 81818 invoked by uid 99); 20 Nov 2014 00:58:03 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 20 Nov 2014 00:58:03 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 1DB97116F50; Thu, 20 Nov 2014 00:58:03 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============6172963520010617542==" MIME-Version: 1.0 Subject: Re: Review Request 27606: Standardized path handling in files From: "Timothy Chen" To: "Ben Mahler" Cc: "Cody Maloney" , "Timothy Chen" , "mesos" Date: Thu, 20 Nov 2014 00:58:03 -0000 Message-ID: <20141120005803.15325.29981@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org Auto-Submitted: auto-generated Sender: "Timothy Chen" X-ReviewGroup: mesos X-ReviewRequest-URL: https://reviews.apache.org/r/27606/ X-Sender: "Timothy Chen" References: <20141120001636.16171.83659@reviews.apache.org> In-Reply-To: <20141120001636.16171.83659@reviews.apache.org> Reply-To: "Timothy Chen" X-ReviewRequest-Repository: mesos-git --===============6172963520010617542== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On Nov. 20, 2014, 12:16 a.m., Timothy Chen wrote: > > src/files/files.cpp, line 151 > > > > > > Why Future? This seems to return right away. > > And it doesn't look like a interface you want to extend too. > > Cody Maloney wrote: > attach() and detach() are called through a dispatch(), this hooks up to that future (See the Files::detach() wrapper which dispatches through libprocess at the end of the file to call this function. > > The primary motivation here for adding a return is so that we can check that attach() and detach() are actually symmettric (Most callers don't care if they weren't able to detach something which doesn't exist). > > Prior to adding standardizePath in this patch there was a whole swath of cases where attach() and detach() would not end up matching eachother given the same path. ok makes sense. - Timothy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27606/#review62298 ----------------------------------------------------------- On Nov. 20, 2014, 12:34 a.m., Cody Maloney wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27606/ > ----------------------------------------------------------- > > (Updated Nov. 20, 2014, 12:34 a.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: mesos-1877 > https://issues.apache.org/jira/browse/mesos-1877 > > > Repository: mesos-git > > > Description > ------- > > Files attach, detach, and browse now all call a single function to standardize the path. Before attach, browse did the normalization. Detach did not. > > Switch to strings::tokenize() for splitting apart the path into tokens, as that is more canonical across the codebase, used by libprocess for proccessing paths. > > Update detach to return a bool of whether or not the detach does anything which means that it can be tested. > > Add some additional testing to catch the inconsistencies in path handlign between attach and detach previously, some of the oddness in current path handling so that they can be explicitly seen when changes happen. > > > Diffs > ----- > > src/files/files.hpp 818087b13cc787d0bd3186bb3e8a069751629bf9 > src/files/files.cpp 12e8f75aa7bd77d2e81d5d3a7a4d09dd915854aa > src/tests/files_tests.cpp 9ad6db51873b96b3cd759523cef9748f6823fb7e > > Diff: https://reviews.apache.org/r/27606/diff/ > > > Testing > ------- > > make distcheck on ubuntu 14.04 > > > Thanks, > > Cody Maloney > > --===============6172963520010617542==--