Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-10-24 Thread Qin Liu
> On Sept. 22, 2016, 7:54 p.m., Robert Levas wrote: > > ambari-metrics/ambari-metrics-host-monitoring/src/main/python/core/spnego_kerberos_auth.py, > > line 33 > > > > > > See `curl_krb_request` > > Dmytro Sen

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-10-18 Thread Aravindan Vijayan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51724/#review153108 --- Ship it! Ship It! - Aravindan Vijayan On Sept. 22, 2016,

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-10-12 Thread Dmytro Sen
> On Сен. 22, 2016, 7:54 п.п., Robert Levas wrote: > > ambari-metrics/ambari-metrics-host-monitoring/src/main/python/core/spnego_kerberos_auth.py, > > line 33 > > > > > > See `curl_krb_request` I think, we don't

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-23 Thread Qin Liu
> On Sept. 8, 2016, 6:13 p.m., Sid Wagle wrote: > > ambari-metrics/ambari-metrics-host-monitoring/src/main/python/core/spnego_kerberos_auth.py, > > line 25 > > > > > > I do not see a "import kerberos" anywhere in

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-22 Thread Qin Liu
> On Sept. 22, 2016, 7:54 p.m., Robert Levas wrote: > > ambari-metrics/ambari-metrics-host-monitoring/src/main/python/core/emitter.py, > > line 121 > > > > > > Possibly use `curl_krb_request` instead. > > Di Li

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-22 Thread Di Li
> On Sept. 22, 2016, 7:54 p.m., Robert Levas wrote: > > ambari-metrics/ambari-metrics-host-monitoring/src/main/python/core/emitter.py, > > line 121 > > > > > > Possibly use `curl_krb_request` instead. Qin, please

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-22 Thread Tim Thorpe
> On Sept. 22, 2016, 7:54 p.m., Robert Levas wrote: > > ambari-metrics/ambari-metrics-host-monitoring/src/main/python/core/config_reader.py, > > lines 119-120 > > > > > > These shoud not be hardcoded. The aths to

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-22 Thread Robert Levas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51724/#review150057 ---

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-22 Thread Qin Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51724/ --- (Updated Sept. 22, 2016, 12:44 p.m.) Review request for Ambari, Di Li, Dmytro

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-22 Thread Qin Liu
> On Sept. 19, 2016, 9:18 a.m., Robert Levas wrote: > > I think this is incorrect. The Metrics Monitor should authenticate with > > its own service principal, not the SPNEGO principal. That is used for > > web-based services, like Ambari's web-based interface. There should be an > >

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-22 Thread Qin Liu
> On Sept. 8, 2016, 6:13 p.m., Sid Wagle wrote: > > ambari-metrics/ambari-metrics-host-monitoring/src/main/python/core/spnego_kerberos_auth.py, > > line 25 > > > > > > I do not see a "import kerberos" anywhere in

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-22 Thread Qin Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51724/ --- (Updated Sept. 22, 2016, 10:14 a.m.) Review request for Ambari, Di Li, Dmytro

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-19 Thread Sid Wagle
> On Sept. 19, 2016, 9:18 a.m., Robert Levas wrote: > > ambari-metrics/ambari-metrics-host-monitoring/src/main/python/core/config_reader.py, > > line 112 > > > > > > The path to the SPNEGO keytab file and the

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-19 Thread Qin Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51724/ --- (Updated Sept. 19, 2016, 3:53 p.m.) Review request for Ambari, Di Li, Dmytro

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-19 Thread Robert Levas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51724/#review149443 --- I think this is incorrect. The Metrics Monitor should

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-18 Thread Sid Wagle
> On Sept. 8, 2016, 6:13 p.m., Sid Wagle wrote: > > ambari-metrics/ambari-metrics-host-monitoring/src/main/python/core/spnego_kerberos_auth.py, > > line 25 > > > > > > I do not see a "import kerberos" anywhere in

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-17 Thread Qin Liu
> On Sept. 8, 2016, 6:13 p.m., Sid Wagle wrote: > > ambari-metrics/ambari-metrics-host-monitoring/src/main/python/core/spnego_kerberos_auth.py, > > line 25 > > > > > > I do not see a "import kerberos" anywhere in

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-12 Thread Aravindan Vijayan
On Sept. 8, 2016, 6:13 p.m., Qin Liu wrote: > > We don't have unit test whcih setup secure cluster so we should definitely > > test this on trunk build, which should be working now. > > The changes overall look good just want to make sure we have done some > > manual testing before commit. >

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-12 Thread Sid Wagle
> On Sept. 8, 2016, 6:13 p.m., Sid Wagle wrote: > > ambari-metrics/ambari-metrics-host-monitoring/src/main/python/core/spnego_kerberos_auth.py, > > line 25 > > > > > > I do not see a "import kerberos" anywhere in

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-12 Thread Qin Liu
On Sept. 8, 2016, 6:13 p.m., Qin Liu wrote: > > We don't have unit test whcih setup secure cluster so we should definitely > > test this on trunk build, which should be working now. > > The changes overall look good just want to make sure we have done some > > manual testing before commit. >

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-09 Thread Qin Liu
> On Sept. 8, 2016, 6:13 p.m., Sid Wagle wrote: > > ambari-metrics/ambari-metrics-host-monitoring/src/main/python/core/emitter.py, > > line 145 > > > > > > Shouldn't this be checking for None instead of empty

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-09 Thread Qin Liu
> On Sept. 8, 2016, 6:13 p.m., Sid Wagle wrote: > > ambari-metrics/ambari-metrics-host-monitoring/src/main/python/core/spnego_kerberos_auth.py, > > line 25 > > > > > > I do not see a "import kerberos" anywhere in

Re: Review Request 51724: Add Kerberos HTTP SPNEGO authentication support to Ambari Metrics Monitor

2016-09-08 Thread Sid Wagle
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51724/#review148224 ---