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/01/09 14:54:24 UTC
Review Request 55342: Modify quick link resource provider to consider
filters and return visibility.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55342/
-----------------------------------------------------------
Review request for Ambari, Attila Doroszlai, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
Bugs: AMBARI-19426
https://issues.apache.org/jira/browse/AMBARI-19426
Repository: ambari
Description
-------
Existing quicklinks API (e.g: http://c6401:8080/api/v1/stacks/HDP/versions/2.5/services/ACCUMULO/quicklinks/quicklinks.json) should return visible = true/false based on the quick links profile.
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java cc20324
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0affa4f
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProvider.java 5603765
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinks/Link.java f589f5d
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityController.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactory.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java 31335b6
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorException.java c24281a
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/ShowAllLinksVisibilityController.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProviderTest.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java f54842d
ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactoryTest.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorTest.java 6a31ca0
ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParserTest.java 6f5dd07
ambari-server/src/test/resources/example_quicklinks_profile.json 2fa33a4
ambari-server/src/test/resources/inconsistent_quicklinks_profile_2.json PRE-CREATION
contrib/version-builder/version_242-12345.xml PRE-CREATION
Diff: https://reviews.apache.org/r/55342/diff/
Testing
-------
1. Did manual testing
2. Wrote new unit tests
3. Run the ambari-server unit test suite. Only KerberosServiceMetaInfoTest failed, but it succeeded in a subsequent run.
Thanks,
Bal�zs Bence S�ri
Re: Review Request 55342: Modify quick link resource provider to
consider filters and return visibility.
Posted by Balázs Bence Sári <bs...@hortonworks.com>.
> On Jan. 11, 2017, 12:13 p.m., Laszlo Puskas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java, line 5529
> > <https://reviews.apache.org/r/55342/diff/3/?file=1600114#file1600114line5529>
> >
> > The factory could be injected; thus it shouldnt b ea utility (with static factory methods)
I wouldn't add complication here. Don't be mislead by the fact that a "factory" is used here. The intention is not to return something configuration based (where Guice would serve perfectly) but to use a strategy that corresponds to the method argument that can be different on each invocation.
It could have been simply an utility method in AmbariManagementControllerImpl, the main reason I chose to factor it out is to avoid adding too much stuff to the already too complex AmbariManagementControllerImpl class.
If a non-Guice factory is a problem, I would rather inline the logic into AmbariManagementControllerImpl than adding more complexity.
> On Jan. 11, 2017, 12:13 p.m., Laszlo Puskas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java, line 45
> > <https://reviews.apache.org/r/55342/diff/3/?file=1600119#file1600119line45>
> >
> > The logic from the constructor could be extracted into "business" methods. (These also could be managed with guice) Unit testing would be also easier.
Please see above. There is nothing configuration dependent here that should be managed by Guice. It is rather an implementation of the strategy pattern.
> On Jan. 11, 2017, 12:13 p.m., Laszlo Puskas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactory.java, line 34
> > <https://reviews.apache.org/r/55342/diff/3/?file=1600118#file1600118line34>
> >
> > Ths class could be a bean managed by the container. Ideally it would be enough to define an interface and register it as factory in the guice context. The logic this factory method performs could be implemented int he callee.
Please see above.
- Bal�zs Bence
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55342/#review161216
-----------------------------------------------------------
On Jan. 9, 2017, 3:07 p.m., Bal�zs Bence S�ri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55342/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2017, 3:07 p.m.)
>
>
> Review request for Ambari, Attila Doroszlai, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
>
>
> Bugs: AMBARI-19426
> https://issues.apache.org/jira/browse/AMBARI-19426
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Existing quicklinks API (e.g: http://c6401:8080/api/v1/stacks/HDP/versions/2.5/services/ACCUMULO/quicklinks/quicklinks.json) should return visible = true/false based on the quick links profile.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java cc20324
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0affa4f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProvider.java 5603765
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinks/Link.java f589f5d
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityController.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactory.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java 31335b6
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorException.java c24281a
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/ShowAllLinksVisibilityController.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProviderTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java f54842d
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactoryTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorTest.java 6a31ca0
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParserTest.java 6f5dd07
> ambari-server/src/test/resources/example_quicklinks_profile.json 2fa33a4
> ambari-server/src/test/resources/inconsistent_quicklinks_profile_2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55342/diff/
>
>
> Testing
> -------
>
> 1. Did manual testing
> 2. Wrote new unit tests
> 3. Run the ambari-server unit test suite. Only KerberosServiceMetaInfoTest failed, but it succeeded in a subsequent run.
>
>
> Thanks,
>
> Bal�zs Bence S�ri
>
>
Re: Review Request 55342: Modify quick link resource provider to
consider filters and return visibility.
Posted by Laszlo Puskas <lp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55342/#review161216
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java (line 5529)
<https://reviews.apache.org/r/55342/#comment232454>
The factory could be injected; thus it shouldnt b ea utility (with static factory methods)
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactory.java (line 34)
<https://reviews.apache.org/r/55342/#comment232456>
Ths class could be a bean managed by the container. Ideally it would be enough to define an interface and register it as factory in the guice context. The logic this factory method performs could be implemented int he callee.
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java (line 45)
<https://reviews.apache.org/r/55342/#comment232458>
The logic from the constructor could be extracted into "business" methods. (These also could be managed with guice) Unit testing would be also easier.
- Laszlo Puskas
On Jan. 9, 2017, 3:07 p.m., Bal�zs Bence S�ri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55342/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2017, 3:07 p.m.)
>
>
> Review request for Ambari, Attila Doroszlai, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
>
>
> Bugs: AMBARI-19426
> https://issues.apache.org/jira/browse/AMBARI-19426
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Existing quicklinks API (e.g: http://c6401:8080/api/v1/stacks/HDP/versions/2.5/services/ACCUMULO/quicklinks/quicklinks.json) should return visible = true/false based on the quick links profile.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java cc20324
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0affa4f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProvider.java 5603765
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinks/Link.java f589f5d
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityController.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactory.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java 31335b6
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorException.java c24281a
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/ShowAllLinksVisibilityController.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProviderTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java f54842d
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactoryTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorTest.java 6a31ca0
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParserTest.java 6f5dd07
> ambari-server/src/test/resources/example_quicklinks_profile.json 2fa33a4
> ambari-server/src/test/resources/inconsistent_quicklinks_profile_2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55342/diff/
>
>
> Testing
> -------
>
> 1. Did manual testing
> 2. Wrote new unit tests
> 3. Run the ambari-server unit test suite. Only KerberosServiceMetaInfoTest failed, but it succeeded in a subsequent run.
>
>
> Thanks,
>
> Bal�zs Bence S�ri
>
>
Re: Review Request 55342: Modify quick link resource provider to
consider filters and return visibility.
Posted by Sebastian Toader <st...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55342/#review161045
-----------------------------------------------------------
Ship it!
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java (line 242)
<https://reviews.apache.org/r/55342/#comment232308>
Document what "QuickLinksProfile" identify
- Sebastian Toader
On Jan. 9, 2017, 4:07 p.m., Bal�zs Bence S�ri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55342/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2017, 4:07 p.m.)
>
>
> Review request for Ambari, Attila Doroszlai, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
>
>
> Bugs: AMBARI-19426
> https://issues.apache.org/jira/browse/AMBARI-19426
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Existing quicklinks API (e.g: http://c6401:8080/api/v1/stacks/HDP/versions/2.5/services/ACCUMULO/quicklinks/quicklinks.json) should return visible = true/false based on the quick links profile.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java cc20324
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0affa4f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProvider.java 5603765
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinks/Link.java f589f5d
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityController.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactory.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java 31335b6
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorException.java c24281a
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/ShowAllLinksVisibilityController.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProviderTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java f54842d
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactoryTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorTest.java 6a31ca0
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParserTest.java 6f5dd07
> ambari-server/src/test/resources/example_quicklinks_profile.json 2fa33a4
> ambari-server/src/test/resources/inconsistent_quicklinks_profile_2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55342/diff/
>
>
> Testing
> -------
>
> 1. Did manual testing
> 2. Wrote new unit tests
> 3. Run the ambari-server unit test suite. Only KerberosServiceMetaInfoTest failed, but it succeeded in a subsequent run.
>
>
> Thanks,
>
> Bal�zs Bence S�ri
>
>
Re: Review Request 55342: Modify quick link resource provider to
consider filters and return visibility.
Posted by Laszlo Puskas <lp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55342/#review161236
-----------------------------------------------------------
Ship it!
Ship It!
- Laszlo Puskas
On Jan. 9, 2017, 3:07 p.m., Bal�zs Bence S�ri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55342/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2017, 3:07 p.m.)
>
>
> Review request for Ambari, Attila Doroszlai, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
>
>
> Bugs: AMBARI-19426
> https://issues.apache.org/jira/browse/AMBARI-19426
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Existing quicklinks API (e.g: http://c6401:8080/api/v1/stacks/HDP/versions/2.5/services/ACCUMULO/quicklinks/quicklinks.json) should return visible = true/false based on the quick links profile.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java cc20324
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0affa4f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProvider.java 5603765
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinks/Link.java f589f5d
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityController.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactory.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java 31335b6
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorException.java c24281a
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/ShowAllLinksVisibilityController.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProviderTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java f54842d
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactoryTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorTest.java 6a31ca0
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParserTest.java 6f5dd07
> ambari-server/src/test/resources/example_quicklinks_profile.json 2fa33a4
> ambari-server/src/test/resources/inconsistent_quicklinks_profile_2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55342/diff/
>
>
> Testing
> -------
>
> 1. Did manual testing
> 2. Wrote new unit tests
> 3. Run the ambari-server unit test suite. Only KerberosServiceMetaInfoTest failed, but it succeeded in a subsequent run.
>
>
> Thanks,
>
> Bal�zs Bence S�ri
>
>
Re: Review Request 55342: Modify quick link resource provider to
consider filters and return visibility.
Posted by Balázs Bence Sári <bs...@hortonworks.com>.
> On Jan. 11, 2017, 4:28 p.m., Attila Doroszlai wrote:
> > ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java, line 113
> > <https://reviews.apache.org/r/55342/diff/3/?file=1600123#file1600123line113>
> >
> > Somehow this comment doesn't seem right. The test verifies the link is visible, not hidden.
The comment is meant for a different test. Probably a result of a former merge.
- Bal�zs Bence
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55342/#review161258
-----------------------------------------------------------
On Jan. 11, 2017, 5:27 p.m., Bal�zs Bence S�ri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55342/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2017, 5:27 p.m.)
>
>
> Review request for Ambari, Attila Doroszlai, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
>
>
> Bugs: AMBARI-19426
> https://issues.apache.org/jira/browse/AMBARI-19426
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Existing quicklinks API (e.g: http://c6401:8080/api/v1/stacks/HDP/versions/2.5/services/ACCUMULO/quicklinks/quicklinks.json) should return visible = true/false based on the quick links profile.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java cc20324
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0affa4f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProvider.java 5603765
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinks/Link.java f589f5d
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityController.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactory.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfile.java e86af38
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java 31335b6
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorException.java c24281a
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/ShowAllLinksVisibilityController.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProviderTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java f54842d
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactoryTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorTest.java 6a31ca0
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParserTest.java 6f5dd07
> ambari-server/src/test/resources/example_quicklinks_profile.json 2fa33a4
> ambari-server/src/test/resources/inconsistent_quicklinks_profile_2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55342/diff/
>
>
> Testing
> -------
>
> 1. Did manual testing
> 2. Wrote new unit tests
> 3. Run the ambari-server unit test suite. Only KerberosServiceMetaInfoTest failed, but it succeeded in a subsequent run.
>
>
> Thanks,
>
> Bal�zs Bence S�ri
>
>
Re: Review Request 55342: Modify quick link resource provider to
consider filters and return visibility.
Posted by Attila Doroszlai <ad...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55342/#review161258
-----------------------------------------------------------
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProviderTest.java (line 76)
<https://reviews.apache.org/r/55342/#comment232498>
Please do not add @throws.
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProviderTest.java (line 102)
<https://reviews.apache.org/r/55342/#comment232499>
Please do not add @throws.
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProviderTest.java (line 121)
<https://reviews.apache.org/r/55342/#comment232496>
"hidden"?
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProviderTest.java (line 127)
<https://reviews.apache.org/r/55342/#comment232500>
Please do not add @throws.
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProviderTest.java (line 146)
<https://reviews.apache.org/r/55342/#comment232497>
hidden?
ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java (line 113)
<https://reviews.apache.org/r/55342/#comment232495>
Somehow this comment doesn't seem right. The test verifies the link is visible, not hidden.
ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorTest.java (line 66)
<https://reviews.apache.org/r/55342/#comment232502>
Please do not add @throws.
- Attila Doroszlai
On Jan. 9, 2017, 4:07 p.m., Bal�zs Bence S�ri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55342/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2017, 4:07 p.m.)
>
>
> Review request for Ambari, Attila Doroszlai, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
>
>
> Bugs: AMBARI-19426
> https://issues.apache.org/jira/browse/AMBARI-19426
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Existing quicklinks API (e.g: http://c6401:8080/api/v1/stacks/HDP/versions/2.5/services/ACCUMULO/quicklinks/quicklinks.json) should return visible = true/false based on the quick links profile.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java cc20324
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0affa4f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProvider.java 5603765
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinks/Link.java f589f5d
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityController.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactory.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java 31335b6
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorException.java c24281a
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/ShowAllLinksVisibilityController.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProviderTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java f54842d
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactoryTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorTest.java 6a31ca0
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParserTest.java 6f5dd07
> ambari-server/src/test/resources/example_quicklinks_profile.json 2fa33a4
> ambari-server/src/test/resources/inconsistent_quicklinks_profile_2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55342/diff/
>
>
> Testing
> -------
>
> 1. Did manual testing
> 2. Wrote new unit tests
> 3. Run the ambari-server unit test suite. Only KerberosServiceMetaInfoTest failed, but it succeeded in a subsequent run.
>
>
> Thanks,
>
> Bal�zs Bence S�ri
>
>
Re: Review Request 55342: Modify quick link resource provider to
consider filters and return visibility.
Posted by Sandor Magyari <sm...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55342/#review161070
-----------------------------------------------------------
Ship it!
Ship It!
- Sandor Magyari
On Jan. 9, 2017, 3:07 p.m., Bal�zs Bence S�ri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55342/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2017, 3:07 p.m.)
>
>
> Review request for Ambari, Attila Doroszlai, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
>
>
> Bugs: AMBARI-19426
> https://issues.apache.org/jira/browse/AMBARI-19426
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Existing quicklinks API (e.g: http://c6401:8080/api/v1/stacks/HDP/versions/2.5/services/ACCUMULO/quicklinks/quicklinks.json) should return visible = true/false based on the quick links profile.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java cc20324
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0affa4f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProvider.java 5603765
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinks/Link.java f589f5d
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityController.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactory.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java 31335b6
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorException.java c24281a
> ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/ShowAllLinksVisibilityController.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProviderTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java f54842d
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactoryTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorTest.java 6a31ca0
> ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParserTest.java 6f5dd07
> ambari-server/src/test/resources/example_quicklinks_profile.json 2fa33a4
> ambari-server/src/test/resources/inconsistent_quicklinks_profile_2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55342/diff/
>
>
> Testing
> -------
>
> 1. Did manual testing
> 2. Wrote new unit tests
> 3. Run the ambari-server unit test suite. Only KerberosServiceMetaInfoTest failed, but it succeeded in a subsequent run.
>
>
> Thanks,
>
> Bal�zs Bence S�ri
>
>
Re: Review Request 55342: Modify quick link resource provider to
consider filters and return visibility.
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/55342/
-----------------------------------------------------------
(Updated Jan. 11, 2017, 5:27 p.m.)
Review request for Ambari, Attila Doroszlai, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
Changes
-------
Fixed review comments.
Bugs: AMBARI-19426
https://issues.apache.org/jira/browse/AMBARI-19426
Repository: ambari
Description
-------
Existing quicklinks API (e.g: http://c6401:8080/api/v1/stacks/HDP/versions/2.5/services/ACCUMULO/quicklinks/quicklinks.json) should return visible = true/false based on the quick links profile.
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java cc20324
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0affa4f
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProvider.java 5603765
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinks/Link.java f589f5d
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityController.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactory.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfile.java e86af38
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java 31335b6
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorException.java c24281a
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/ShowAllLinksVisibilityController.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProviderTest.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java f54842d
ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactoryTest.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorTest.java 6a31ca0
ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParserTest.java 6f5dd07
ambari-server/src/test/resources/example_quicklinks_profile.json 2fa33a4
ambari-server/src/test/resources/inconsistent_quicklinks_profile_2.json PRE-CREATION
Diff: https://reviews.apache.org/r/55342/diff/
Testing
-------
1. Did manual testing
2. Wrote new unit tests
3. Run the ambari-server unit test suite. Only KerberosServiceMetaInfoTest failed, but it succeeded in a subsequent run.
Thanks,
Bal�zs Bence S�ri
Re: Review Request 55342: Modify quick link resource provider to
consider filters and return visibility.
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/55342/
-----------------------------------------------------------
(Updated Jan. 9, 2017, 3:07 p.m.)
Review request for Ambari, Attila Doroszlai, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
Bugs: AMBARI-19426
https://issues.apache.org/jira/browse/AMBARI-19426
Repository: ambari
Description
-------
Existing quicklinks API (e.g: http://c6401:8080/api/v1/stacks/HDP/versions/2.5/services/ACCUMULO/quicklinks/quicklinks.json) should return visible = true/false based on the quick links profile.
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java cc20324
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0affa4f
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProvider.java 5603765
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinks/Link.java f589f5d
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityController.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactory.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java 31335b6
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorException.java c24281a
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/ShowAllLinksVisibilityController.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProviderTest.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java f54842d
ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactoryTest.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorTest.java 6a31ca0
ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParserTest.java 6f5dd07
ambari-server/src/test/resources/example_quicklinks_profile.json 2fa33a4
ambari-server/src/test/resources/inconsistent_quicklinks_profile_2.json PRE-CREATION
Diff: https://reviews.apache.org/r/55342/diff/
Testing
-------
1. Did manual testing
2. Wrote new unit tests
3. Run the ambari-server unit test suite. Only KerberosServiceMetaInfoTest failed, but it succeeded in a subsequent run.
Thanks,
Bal�zs Bence S�ri
Re: Review Request 55342: Modify quick link resource provider to
consider filters and return visibility.
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/55342/
-----------------------------------------------------------
(Updated Jan. 9, 2017, 3 p.m.)
Review request for Ambari, Attila Doroszlai, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
Bugs: AMBARI-19426
https://issues.apache.org/jira/browse/AMBARI-19426
Repository: ambari
Description
-------
Existing quicklinks API (e.g: http://c6401:8080/api/v1/stacks/HDP/versions/2.5/services/ACCUMULO/quicklinks/quicklinks.json) should return visible = true/false based on the quick links profile.
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java cc20324
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0affa4f
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProvider.java 5603765
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinks/Link.java f589f5d
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityController.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactory.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluator.java 31335b6
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorException.java c24281a
ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/ShowAllLinksVisibilityController.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/QuickLinkArtifactResourceProviderTest.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/EvaluatorTest.java f54842d
ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinkVisibilityControllerFactoryTest.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileEvaluatorTest.java 6a31ca0
ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParserTest.java 6f5dd07
ambari-server/src/test/resources/example_quicklinks_profile.json 2fa33a4
ambari-server/src/test/resources/inconsistent_quicklinks_profile_2.json PRE-CREATION
Diff: https://reviews.apache.org/r/55342/diff/
Testing
-------
1. Did manual testing
2. Wrote new unit tests
3. Run the ambari-server unit test suite. Only KerberosServiceMetaInfoTest failed, but it succeeded in a subsequent run.
Thanks,
Bal�zs Bence S�ri