----------------------------------------------------------- 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 > >
