Return-Path: X-Original-To: apmail-ambari-dev-archive@www.apache.org Delivered-To: apmail-ambari-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 C6A34CBD9 for ; Fri, 19 Dec 2014 06:30:08 +0000 (UTC) Received: (qmail 40690 invoked by uid 500); 19 Dec 2014 06:30:08 -0000 Delivered-To: apmail-ambari-dev-archive@ambari.apache.org Received: (qmail 40660 invoked by uid 500); 19 Dec 2014 06:30:08 -0000 Mailing-List: contact dev-help@ambari.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@ambari.apache.org Delivered-To: mailing list dev@ambari.apache.org Received: (qmail 40616 invoked by uid 99); 19 Dec 2014 06:30:07 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 19 Dec 2014 06:30:07 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id F38641D2C93; Fri, 19 Dec 2014 06:29:59 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============2982949534088774625==" MIME-Version: 1.0 Subject: Re: Review Request 29009: Refactor the OS-dependent Ambari Server Windows components - part 1 From: "Florian Barca" To: "Mahadev Konar" , "Jayush Luniya" , "Artem Baranchuk" , "Eugene Chekanskiy" , "Jonathan Hurley" , "Nate Cole" Cc: "Florian Barca" , "Ambari" Date: Fri, 19 Dec 2014 06:29:59 -0000 Message-ID: <20141219062959.12934.65814@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org Auto-Submitted: auto-generated Sender: "Florian Barca" X-ReviewGroup: Ambari X-ReviewRequest-URL: https://reviews.apache.org/r/29009/ X-Sender: "Florian Barca" References: <20141219054731.12935.67530@reviews.apache.org> In-Reply-To: <20141219054731.12935.67530@reviews.apache.org> Reply-To: "Florian Barca" X-ReviewRequest-Repository: ambari --===============2982949534088774625== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On Dec. 19, 2014, 5:47 a.m., Jayush Luniya wrote: > > ambari-common/src/main/python/ambari_commons/os_utils.py, line 96 > > > > > > Ahh, why do we need a run_os_command which calls os_run_os_command? We need to be able to call an OS-independent abstraction without importing the OS-dependent declarations. With run_os_command placed directly in os_linux/os_windows, there was no way to achieve that. We also need to keep the compatibility with the existing code, so the name run_os_command needs to be perpetuated with its original intent. For all these reasons, run_os_command is indeed a thin wrapper over the OS-dependent implementation. In this particular spot, we can afford to conditionally import declarations from the OS-dependent scripts. > On Dec. 19, 2014, 5:47 a.m., Jayush Luniya wrote: > > ambari-server/pom.xml, line 1084 > > > > > > Do we need to hardcode this? What if someone wants to set the limit as 1024m or 4096m? The initial motivation for having this setting was that the builds & tests started failing on my VM with PermGen space error. Adding this argument was the only reliable way to keep the build going. Whether this limit is enough, is a matter of debate. For me, 1024m work fine. If you find yourself getting PermGen errors even with 2048m, feel free to increase the limit. > On Dec. 19, 2014, 5:47 a.m., Jayush Luniya wrote: > > ambari-common/src/main/python/ambari_commons/os_linux.py, line 36 > > > > > > Nit. os_run_os_command? run_os_command sounds better See my comment below about os_run_os_command. - Florian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29009/#review65608 ----------------------------------------------------------- On Dec. 18, 2014, 4:51 a.m., Florian Barca wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29009/ > ----------------------------------------------------------- > > (Updated Dec. 18, 2014, 4:51 a.m.) > > > Review request for Ambari, Artem Baranchuk, Eugene Chekanskiy, Jonathan Hurley, Jayush Luniya, Mahadev Konar, and Nate Cole. > > > Bugs: AMBARI-8373 > https://issues.apache.org/jira/browse/AMBARI-8373 > > > Repository: ambari > > > Description > ------- > > Refactored the Ambari Server setup+reset+update. > > +Pervasively adopted the OS-independent factory-based object model > +Unwired the setup-related code from ambari-server.py into separate files. > +Fixed the unit tests > > > Diffs > ----- > > ambari-agent/src/main/python/ambari_agent/HostInfo.py a99a85dc0e9a6a9e13c7ccc94741da0b83bb0710 > ambari-common/src/main/python/ambari_commons/firewall.py b73cc0cc15cb441b35c0db7ed449ee8f2440134f > ambari-common/src/main/python/ambari_commons/inet_utils.py 2a54cb6867aec65d6400451ab12e30c146193cfa > ambari-common/src/main/python/ambari_commons/os_family_impl.py f8a3379df35a7ff539ca67f213c8512c761b9c60 > ambari-common/src/main/python/ambari_commons/os_linux.py 38f3fb9999d19056cca471ad7de64a4b62cb9723 > ambari-common/src/main/python/ambari_commons/os_utils.py 3f4819d0b0306fd1bba09f0772c0a5cf458def60 > ambari-common/src/main/python/ambari_commons/os_windows.py 2fb98e49478640ebfbabb820683c83331b7ed510 > ambari-common/src/main/python/resource_management/core/providers/windows/system.py e7a98fc432003b7b818671b8c7fdcfc498f3ad84 > ambari-common/src/main/python/resource_management/libraries/functions/get_unique_id_and_date.py afc82bb4905c806a16a6b67831fdf2fc02ed0875 > ambari-server/conf/unix/ambari.properties e29a6e25f576a8f806a447a4cf03c56f4d0c105b > ambari-server/conf/windows/ambari.properties 3982bb9d6ccde94e7548b121bb4cfff5fabd3977 > ambari-server/pom.xml 78012ba63978b2d64925c4ef5d20554286f6766f > ambari-server/src/main/python/ambari-server-windows.py 6c4f894de0f6f4abb90b26ebb36a1825d19f9b0f > ambari-server/src/main/python/ambari-server.py 89caa2ea4caa726f206bdfbde03ae95a6af0f082 > ambari-server/src/main/python/ambari_server/dbConfiguration.py daa84abbfe507aafacb4f5e4f81be3864a2ea0e6 > ambari-server/src/main/python/ambari_server/dbConfiguration_linux.py ce47faec7023050adc451d6df5c9f6cac64ec9be > ambari-server/src/main/python/ambari_server/dbConfiguration_windows.py 647a940ae598e783194e7d0a00fe91f0e080ff48 > ambari-server/src/main/python/ambari_server/properties.py 8e00762d8158d4351544d0a1a88350bdc722332f > ambari-server/src/main/python/ambari_server/resourceFilesKeeper.py 9d138a192131dc79e2a028ea4b8aa10983c751c6 > ambari-server/src/main/python/ambari_server/serverConfiguration.py aab116711219f633e3a1a6328d80e9ec68308d15 > ambari-server/src/main/python/ambari_server/serverConfiguration_linux.py a21437a8372c6fdeef01bc9867dee601a029fc02 > ambari-server/src/main/python/ambari_server/serverConfiguration_windows.py a0fa508353110afd7a921aa4156f3e2f897d48f8 > ambari-server/src/main/python/ambari_server/serverSetup.py 37e46f5a480a6186a42a49ce64637da76b133b48 > ambari-server/src/main/python/ambari_server/serverSetup_linux.py b5436e0c1263e52b1ac9b33a24f02e969c72cd70 > ambari-server/src/main/python/ambari_server/serverSetup_windows.py a906ef50020e4eb0b9608bf8c1f43681029699ff > ambari-server/src/main/python/ambari_server/setupSecurity.py 57a323103d23f3d05450a94d9875694001544337 > ambari-server/src/main/python/ambari_server/userInput.py 7a35831d0f24889b5dd29da2445c877d0d1844ca > ambari-server/src/main/resources/custom_actions/scripts/check_host.py 7430ba1954f4efcb51873403bb8dd14a78ae4939 > ambari-server/src/test/python/TestAmbariServer.py 0eae33b2c4a50695cca3fb33fd72065d7f57f4ee > ambari-server/src/test/python/custom_actions/TestCheckHost.py eb738d221a7b67f403c9bc7bfb46490377c2ee1b > > Diff: https://reviews.apache.org/r/29009/diff/ > > > Testing > ------- > > mvn clean test on CentOS > > > Thanks, > > Florian Barca > > --===============2982949534088774625==--