You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Balázs Bence Sári <bs...@hortonworks.com> on 2017/07/21 12:21:57 UTC
Review Request 61024: Implement support for recommending LDAP
configuration
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61024/
-----------------------------------------------------------
Review request for Ambari, Laszlo Puskas, Robert Levas, Sandor Magyari, and Sebastian Toader.
Bugs: AMBARI-21545
https://issues.apache.org/jira/browse/AMBARI-21545
Repository: ambari
Description
-------
Ldap configuration can be included in recommendation requests and validation requests. Configuration is passed down to stack advisor in services.json
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationRequestSwagger.java d6714f9
ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationService.java 0632361
ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRequest.java 7ba1b18
ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java 356754d
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java 2eed23d
ambari-server/src/main/resources/properties.json 11ca7f6
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java 6df8b8b
Diff: https://reviews.apache.org/r/61024/diff/1/
Testing
-------
- Tested the rest interface manually
- Wrote new unit tests
- Ambari-server unit tests: PENDING
Thanks,
Balázs Bence Sári
Re: Review Request 61024: Implement support for recommending LDAP
configuration
Posted by Balázs Bence Sári <bs...@hortonworks.com>.
> On July 21, 2017, 12:41 p.m., Sebastian Toader wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java
> > Lines 81 (patched)
> > <https://reviews.apache.org/r/61024/diff/1/?file=1780662#file1780662line81>
> >
> > Can this be placed to a common place and avoid defining it in two places?
After the rewrite the property is only used in StackAdvisorCommand and its test.
> On July 21, 2017, 12:41 p.m., Sebastian Toader wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java
> > Lines 345 (patched)
> > <https://reviews.apache.org/r/61024/diff/1/?file=1780662#file1780662line345>
> >
> > Add some comment what this method does.
Methdod was deleted along with the rewrite.
- Balázs Bence
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61024/#review181111
-----------------------------------------------------------
On Aug. 8, 2017, 1:40 p.m., Balázs Bence Sári wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61024/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2017, 1:40 p.m.)
>
>
> Review request for Ambari, Laszlo Puskas, Robert Levas, Sandor Magyari, and Sebastian Toader.
>
>
> Bugs: AMBARI-21545
> https://issues.apache.org/jira/browse/AMBARI-21545
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Ldap configuration can be included in recommendation requests and validation requests. Configuration is passed down to stack advisor in services.json
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationRequestSwagger.java d6714f9
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationService.java 0632361
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRequest.java 7ba1b18
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java 356754d
> ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommandTest.java eaa4716
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java 6df8b8b
>
>
> Diff: https://reviews.apache.org/r/61024/diff/2/
>
>
> Testing
> -------
>
> - Tested the rest interface manually
> - Wrote new unit tests
> - Ambari-server unit tests: PENDING
>
>
> Thanks,
>
> Balázs Bence Sári
>
>
Re: Review Request 61024: Implement support for recommending LDAP
configuration
Posted by Sebastian Toader <st...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61024/#review181111
-----------------------------------------------------------
Fix it, then Ship it!
Ship It!
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java
Lines 81 (patched)
<https://reviews.apache.org/r/61024/#comment256534>
Can this be placed to a common place and avoid defining it in two places?
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java
Lines 345 (patched)
<https://reviews.apache.org/r/61024/#comment256535>
Add some comment what this method does.
- Sebastian Toader
On July 21, 2017, 2:21 p.m., Balázs Bence Sári wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61024/
> -----------------------------------------------------------
>
> (Updated July 21, 2017, 2:21 p.m.)
>
>
> Review request for Ambari, Laszlo Puskas, Robert Levas, Sandor Magyari, and Sebastian Toader.
>
>
> Bugs: AMBARI-21545
> https://issues.apache.org/jira/browse/AMBARI-21545
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Ldap configuration can be included in recommendation requests and validation requests. Configuration is passed down to stack advisor in services.json
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationRequestSwagger.java d6714f9
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationService.java 0632361
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRequest.java 7ba1b18
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java 356754d
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java 2eed23d
> ambari-server/src/main/resources/properties.json 11ca7f6
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java 6df8b8b
>
>
> Diff: https://reviews.apache.org/r/61024/diff/1/
>
>
> Testing
> -------
>
> - Tested the rest interface manually
> - Wrote new unit tests
> - Ambari-server unit tests: PENDING
>
>
> Thanks,
>
> Balázs Bence Sári
>
>
Re: Review Request 61024: Implement support for recommending LDAP
configuration
Posted by Laszlo Puskas <lp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61024/#review182388
-----------------------------------------------------------
Ship it!
Ship It!
- Laszlo Puskas
On Aug. 8, 2017, 1:40 p.m., Balázs Bence Sári wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61024/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2017, 1:40 p.m.)
>
>
> Review request for Ambari, Laszlo Puskas, Robert Levas, Sandor Magyari, and Sebastian Toader.
>
>
> Bugs: AMBARI-21545
> https://issues.apache.org/jira/browse/AMBARI-21545
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Ldap configuration can be included in recommendation requests and validation requests. Configuration is passed down to stack advisor in services.json
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationRequestSwagger.java d6714f9
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationService.java 0632361
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRequest.java 7ba1b18
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java 356754d
> ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommandTest.java eaa4716
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java 6df8b8b
>
>
> Diff: https://reviews.apache.org/r/61024/diff/2/
>
>
> Testing
> -------
>
> - Tested the rest interface manually
> - Wrote new unit tests
> - Ambari-server unit tests: PENDING
>
>
> Thanks,
>
> Balázs Bence Sári
>
>
Re: Review Request 61024: Implement support for recommending LDAP
configuration
Posted by Balázs Bence Sári <bs...@hortonworks.com>.
> On Aug. 8, 2017, 8:01 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java
> > Lines 165 (patched)
> > <https://reviews.apache.org/r/61024/diff/3/?file=1793035#file1793035line165>
> >
> > What are the performance implications?
> >
> > Is this invoked on every call?
It has to be invoked in every call. It takes 1-2 millis in my local environment.
> On Aug. 8, 2017, 8:01 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java
> > Lines 184 (patched)
> > <https://reviews.apache.org/r/61024/diff/3/?file=1793035#file1793035line184>
> >
> > Why is this issuing an HTTP call instead of calling the method directly to make it more efficient?
It is not going through http, but calls the rest service endpoint. I copied the existing style of similar methods (getServicesInformation and getHostsInformation) in the class. Performance impact is negligible.
- Balázs Bence
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61024/#review182428
-----------------------------------------------------------
On Aug. 8, 2017, 5:57 p.m., Balázs Bence Sári wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61024/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2017, 5:57 p.m.)
>
>
> Review request for Ambari, Laszlo Puskas, Robert Levas, Sandor Magyari, and Sebastian Toader.
>
>
> Bugs: AMBARI-21545
> https://issues.apache.org/jira/browse/AMBARI-21545
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Ldap configuration can be included in recommendation requests and validation requests. Configuration is passed down to stack advisor in services.json
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationRequestSwagger.java d6714f9
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationService.java 0632361
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRequest.java 7ba1b18
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java 356754d
> ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommandTest.java eaa4716
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java 6df8b8b
>
>
> Diff: https://reviews.apache.org/r/61024/diff/3/
>
>
> Testing
> -------
>
> - Tested the rest interface manually
> - Wrote new unit tests
> - Ambari-server unit tests: PENDING
>
>
> Thanks,
>
> Balázs Bence Sári
>
>
Re: Review Request 61024: Implement support for recommending LDAP
configuration
Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61024/#review182428
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java
Lines 165 (patched)
<https://reviews.apache.org/r/61024/#comment258333>
What are the performance implications?
Is this invoked on every call?
ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java
Lines 184 (patched)
<https://reviews.apache.org/r/61024/#comment258334>
Why is this issuing an HTTP call instead of calling the method directly to make it more efficient?
- Alejandro Fernandez
On Aug. 8, 2017, 5:57 p.m., Balázs Bence Sári wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61024/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2017, 5:57 p.m.)
>
>
> Review request for Ambari, Laszlo Puskas, Robert Levas, Sandor Magyari, and Sebastian Toader.
>
>
> Bugs: AMBARI-21545
> https://issues.apache.org/jira/browse/AMBARI-21545
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Ldap configuration can be included in recommendation requests and validation requests. Configuration is passed down to stack advisor in services.json
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationRequestSwagger.java d6714f9
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationService.java 0632361
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRequest.java 7ba1b18
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java 356754d
> ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommandTest.java eaa4716
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java 6df8b8b
>
>
> Diff: https://reviews.apache.org/r/61024/diff/3/
>
>
> Testing
> -------
>
> - Tested the rest interface manually
> - Wrote new unit tests
> - Ambari-server unit tests: PENDING
>
>
> Thanks,
>
> Balázs Bence Sári
>
>
Re: Review Request 61024: Implement support for recommending LDAP
configuration
Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61024/#review182881
-----------------------------------------------------------
Ship it!
Ship It!
- Alejandro Fernandez
On Aug. 8, 2017, 5:57 p.m., Balázs Bence Sári wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61024/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2017, 5:57 p.m.)
>
>
> Review request for Ambari, Laszlo Puskas, Robert Levas, Sandor Magyari, and Sebastian Toader.
>
>
> Bugs: AMBARI-21545
> https://issues.apache.org/jira/browse/AMBARI-21545
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Ldap configuration can be included in recommendation requests and validation requests. Configuration is passed down to stack advisor in services.json
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationRequestSwagger.java d6714f9
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationService.java 0632361
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRequest.java 7ba1b18
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java 356754d
> ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommandTest.java eaa4716
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java 6df8b8b
>
>
> Diff: https://reviews.apache.org/r/61024/diff/3/
>
>
> Testing
> -------
>
> - Tested the rest interface manually
> - Wrote new unit tests
> - Ambari-server unit tests: PENDING
>
>
> Thanks,
>
> Balázs Bence Sári
>
>
Re: Review Request 61024: Implement support for recommending LDAP
configuration
Posted by Balázs Bence Sári <bs...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61024/
-----------------------------------------------------------
(Updated Aug. 8, 2017, 5:57 p.m.)
Review request for Ambari, Laszlo Puskas, Robert Levas, Sandor Magyari, and Sebastian Toader.
Changes
-------
Added one more unit test.
Bugs: AMBARI-21545
https://issues.apache.org/jira/browse/AMBARI-21545
Repository: ambari
Description
-------
Ldap configuration can be included in recommendation requests and validation requests. Configuration is passed down to stack advisor in services.json
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationRequestSwagger.java d6714f9
ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationService.java 0632361
ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRequest.java 7ba1b18
ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java 356754d
ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommandTest.java eaa4716
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java 6df8b8b
Diff: https://reviews.apache.org/r/61024/diff/3/
Changes: https://reviews.apache.org/r/61024/diff/2-3/
Testing
-------
- Tested the rest interface manually
- Wrote new unit tests
- Ambari-server unit tests: PENDING
Thanks,
Balázs Bence Sári
Re: Review Request 61024: Implement support for recommending LDAP
configuration
Posted by Balázs Bence Sári <bs...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61024/
-----------------------------------------------------------
(Updated Aug. 8, 2017, 1:40 p.m.)
Review request for Ambari, Laszlo Puskas, Robert Levas, Sandor Magyari, and Sebastian Toader.
Changes
-------
LDAP Config is retrieved from server within StackAdvisorCommand. In the previous version the LDAP config was sent with other configs with RecommendationRequest / ConfigurationRequest
Bugs: AMBARI-21545
https://issues.apache.org/jira/browse/AMBARI-21545
Repository: ambari
Description
-------
Ldap configuration can be included in recommendation requests and validation requests. Configuration is passed down to stack advisor in services.json
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationRequestSwagger.java d6714f9
ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationService.java 0632361
ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRequest.java 7ba1b18
ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java 356754d
ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommandTest.java eaa4716
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java 6df8b8b
Diff: https://reviews.apache.org/r/61024/diff/2/
Changes: https://reviews.apache.org/r/61024/diff/1-2/
Testing
-------
- Tested the rest interface manually
- Wrote new unit tests
- Ambari-server unit tests: PENDING
Thanks,
Balázs Bence Sári
Re: Review Request 61024: Implement support for recommending LDAP
configuration
Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61024/#review181112
-----------------------------------------------------------
Ship it!
Ship It!
- Robert Levas
On July 21, 2017, 8:21 a.m., Balázs Bence Sári wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61024/
> -----------------------------------------------------------
>
> (Updated July 21, 2017, 8:21 a.m.)
>
>
> Review request for Ambari, Laszlo Puskas, Robert Levas, Sandor Magyari, and Sebastian Toader.
>
>
> Bugs: AMBARI-21545
> https://issues.apache.org/jira/browse/AMBARI-21545
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Ldap configuration can be included in recommendation requests and validation requests. Configuration is passed down to stack advisor in services.json
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationRequestSwagger.java d6714f9
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariConfigurationService.java 0632361
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRequest.java 7ba1b18
> ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java 356754d
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java 2eed23d
> ambari-server/src/main/resources/properties.json 11ca7f6
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java 6df8b8b
>
>
> Diff: https://reviews.apache.org/r/61024/diff/1/
>
>
> Testing
> -------
>
> - Tested the rest interface manually
> - Wrote new unit tests
> - Ambari-server unit tests: PENDING
>
>
> Thanks,
>
> Balázs Bence Sári
>
>