> On Jan. 7, 2015, 3:23 p.m., Robert Levas wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackArtifactResourceProvider.java, > > line 91 > > <https://reviews.apache.org/r/29664/diff/1/?file=808794#file808794line91> > > > > It seems like this should be `private` especially since is it not an > > immutable set of string and there is a `protected` member method to get > > this data. > > John Speidel wrote: > I absolutely agree that it should be private and it originally was but > some existing unit tests broke because they needed access to these fields. > See ClusterControllerImplTest. These tests use a static test resource and > since we don't have an instance of the actual resource provider, we can't use > it's accessors. I realize that this is really ugly but drew a line on > changing this mechanism and just followed along even though it did cause me > some pain to make them public. > I could have copied the values but thought it more brittle to copy/paste > the set. > The other resource providers that I looked at did the same thing, so I > guess I just gave in. Since I am not the only one that this bothers, and I > am glad about this, I will make them private again and copy/paste the sets as > appropriate for these tests or get the data another way. > > Would you be ok with me making this change in my next patch which is for > the other artifact resource providers?
sure thing. - Robert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29664/#review67071 ----------------------------------------------------------- On Jan. 7, 2015, 2:02 p.m., John Speidel wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29664/ > ----------------------------------------------------------- > > (Updated Jan. 7, 2015, 2:02 p.m.) > > > Review request for Ambari, Nate Cole, Robert Levas, and Tom Beerbower. > > > Bugs: AMBARI-9028 > https://issues.apache.org/jira/browse/AMBARI-9028 > > > Repository: ambari > > > Description > ------- > > Create new API endpoints for exposing stack and stack service kerberos > descriptors. These endpoints will provide the static descriptors which are > defined in the stack definition and will be immutable. > There will be at most 1 descriptor for both the stack and each stack service. > Therefore, it doesn't make sense to add a kerberos_descriptor sub-resource > since there will be at most one instance. > Instead, a new "artifacts" endpoint is being introduced. > The kerberos descriptor will be an artifact instance. > > See the associated Jira for more information. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java > e55b2cb > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/StackServiceResourceDefinition.java > cb1dfb1 > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/StackVersionResourceDefinition.java > e8d4bd5 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/StacksService.java > cdadfd9 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java > 11b0411 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackArtifactResourceProvider.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java > 89d6837 > > ambari-server/src/test/java/org/apache/ambari/server/api/query/QueryImplTest.java > 57aea49 > > ambari-server/src/test/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImplTest.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/api/resources/StackServiceResourceDefinitionTest.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/api/resources/StackVersionResourceDefinitionTest.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/api/services/StacksServiceTest.java > 212eaa0 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProviderTest.java > 5414b52 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterControllerImplTest.java > 3ef0ade > > Diff: https://reviews.apache.org/r/29664/diff/ > > > Testing > ------- > > Manual Functional Testing: > queried new resources including collection and instance resources, queries, > partial response and bad requests > > Unit Testing: > All Java unit tests pass > Results : > > Tests run: 2492, Failures: 0, Errors: 0, Skipped: 13 > > Many python test failures which seem unrelated to my patch. > I have sent an email to dev@ambari to determine if these are known issues or > if I need to investigate. > > Also, no unit test was written for the new StackArtifactResourceProvider due > to time constraints. > I have filed the following Jira to track this and ensure that the tests are > written: > https://issues.apache.org/jira/browse/AMBARI-9026 > > > Thanks, > > John Speidel > >
