Mailing-List: contact dev-help@ambari.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@ambari.apache.org
Content-Type: multipart/alternative;
 boundary="===============0924590312285510732=="
MIME-Version: 1.0
Subject: Re: Review Request 26194: Admin : ambari-server sync-ldap - support
 --existing, --users & --groups options
From: "Jonathan Hurley" <jhurley@hortonworks.com>
To: "Nate Cole" <ncole@hortonworks.com>,
 "Jonathan Hurley" <jhurley@hortonworks.com>
Cc: "Tom Beerbower" <tbeerbower@hortonworks.com>,
 "Ambari" <dev@ambari.apache.org>
Date: Wed, 01 Oct 2014 02:38:34 -0000
Message-ID: <20141001023834.19177.18142@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Jonathan Hurley" <noreply@reviews.apache.org>
References: <20140930213135.18846.89382@reviews.apache.org>
In-Reply-To: <20140930213135.18846.89382@reviews.apache.org>
Reply-To: "Jonathan Hurley" <jhurley@hortonworks.com>

--===============0924590312285510732==
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/26194/#review55047
-----------------------------------------------------------

Ship it!


Overall just nits; code looks straighforward.


ambari-server/src/main/python/ambari-server.py
<https://reviews.apache.org/r/26194/#comment95387>

    get_ldap_event_spec_names doesn't try/catch the parsing of the supplied files. A malformed or unreadable file could cause an exception. Should get_ldap_event_spec_names be wrapped in a try/catch?


ambari-server/src/main/python/ambari-server.py
<https://reviews.apache.org/r/26194/#comment95386>

    Python's documentation says that you don't need to supply a separator, but it implies it uses spaces to determine the splits.
    
    The comment mentions that it's a CSV; should we be explicitly using a comma here? This way we're not at the mercy of an interal splitting algorithm change in a future Python version.


- Jonathan Hurley


On Sept. 30, 2014, 5:31 p.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26194/
> -----------------------------------------------------------
> 
> (Updated Sept. 30, 2014, 5:31 p.m.)
> 
> 
> Review request for Ambari, Jonathan Hurley and Nate Cole.
> 
> 
> Bugs: AMBARI-7569
>     https://issues.apache.org/jira/browse/AMBARI-7569
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The command line 'ambari-server sync-ldap' should support the options as follows ...
> 
> {quote}
> "ambari-server sync-ldap --existing" which would would first look at the ambari database and only clean-up those users and groups. This way, it doesn't pull any new ones, it a) removes users that are in ambari but out of ldap, b) remove groups that are in ambari but out of ldap and c) syncs the group membership of the groups that are in ambari
> 
> "ambari-server sync-ldap --users users.txt --groups groups.txt" which would allow people to just sync a specific set of users and groups from ldap to ambari. The .txt files would be the user and group ids to sync into ambari. This is going to be important for people that just want to bring "a specific set" of users + groups into ambari from ldap (not the "all"). And then once this has happened, running with --existing option periodically helps keep these "in sync"
> {quote}
> 
> 
> Diffs
> -----
> 
>   ambari-server/sbin/ambari-server 027bf87 
>   ambari-server/src/main/python/ambari-server.py 9059319 
>   ambari-server/src/test/python/TestAmbariServer.py 7f769f1 
> 
> Diff: https://reviews.apache.org/r/26194/diff/
> 
> 
> Testing
> -------
> 
> Results :
> 
> Tests run: 2076, Failures: 0, Errors: 0, Skipped: 16
> 
> ...
> 
> [INFO] Executed tasks
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 33:19.993s
> [INFO] Finished at: Tue Sep 30 17:15:25 EDT 2014
> [INFO] Final Memory: 38M/293M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>


--===============0924590312285510732==--