From reviews-return-91744-apmail-mesos-reviews-archive=mesos.apache.org@mesos.apache.org Tue Apr 28 19:14:42 2020 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 [207.244.88.153]) by minotaur.apache.org (Postfix) with SMTP id 8A4E619667 for ; Tue, 28 Apr 2020 19:14:42 +0000 (UTC) Received: (qmail 51293 invoked by uid 500); 28 Apr 2020 19:14:42 -0000 Delivered-To: apmail-mesos-reviews-archive@mesos.apache.org Received: (qmail 51277 invoked by uid 500); 28 Apr 2020 19:14:42 -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 51260 invoked by uid 99); 28 Apr 2020 19:14:41 -0000 Received: from mailrelay1-us-west.apache.org (HELO mailrelay1-us-west.apache.org) (209.188.14.139) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 28 Apr 2020 19:14:41 +0000 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 04BB4E00B7; Tue, 28 Apr 2020 19:14:41 +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 A1926C4024C; Tue, 28 Apr 2020 19:14:40 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============4723745526961057642==" MIME-Version: 1.0 Subject: Re: Review Request 72401: Updated Docker containerizer by not updating resources for command task. From: Greg Mann To: Andrei Budnik , Greg Mann Cc: Qian Zhang , mesos Date: Tue, 28 Apr 2020 19:14:40 -0000 Message-ID: <20200428191440.19515.80709@reviews-vm2.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: Greg Mann X-ReviewGroup: mesos X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/72401/ X-Sender: Greg Mann References: <20200422083312.39403.72149@reviews-vm2.apache.org> In-Reply-To: <20200422083312.39403.72149@reviews-vm2.apache.org> Reply-To: Greg Mann X-ReviewRequest-Repository: mesos --===============4723745526961057642== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72401/#review220529 ----------------------------------------------------------- Could you update the description of the patch to describe the difference between the two cases of a command task and a task with a Docker executor container? src/slave/containerizer/docker.hpp Lines 357 (patched) Nit: you could use the `? :` ternary operator to initialize `generatedForCommandTask` here in the initializer list, I don't think it matters too much but it might be more readable if somebody just glances at the initializer list to see what the value is initialized to. - Greg Mann On April 22, 2020, 8:33 a.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72401/ > ----------------------------------------------------------- > > (Updated April 22, 2020, 8:33 a.m.) > > > Review request for mesos, Andrei Budnik and Greg Mann. > > > Bugs: MESOS-8877 > https://issues.apache.org/jira/browse/MESOS-8877 > > > Repository: mesos > > > Description > ------- > > Updated Docker containerizer by not updating resources for command task. > > > Diffs > ----- > > src/slave/containerizer/docker.hpp 09fc2795289e1292134b7d4bb2cc079c80d91c3d > src/slave/containerizer/docker.cpp 492ac273fafb55d3e4c90d70fbf9c8c0c4fe7e70 > src/tests/containerizer/docker_containerizer_tests.cpp b069f518d51225c39f0cd5126c02fa08674566ab > > > Diff: https://reviews.apache.org/r/72401/diff/2/ > > > Testing > ------- > > > Thanks, > > Qian Zhang > > --===============4723745526961057642==--