----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56463/#review165171 -----------------------------------------------------------
Fix it, then Ship it! Just a couple minor comments. authorization/src/main/java/org/apache/atlas/authorize/simple/PolicyUtil.java (line 125) <https://reviews.apache.org/r/56463/#comment236981> Is it worth documenting this new behavior anywhere? authorization/src/main/java/org/apache/atlas/authorize/simple/PolicyUtil.java (line 160) <https://reviews.apache.org/r/56463/#comment236978> Should we use the Thread's context classloader here? webapp/src/main/java/org/apache/atlas/web/dao/UserDao.java (line 66) <https://reviews.apache.org/r/56463/#comment236979> This seems to largely duplicate the logic above, and my comment about the classloader applies here as well. Can the logic be moved to some utility method? It seems like the only thing different is the names of the properties being used and the default value. - Jeff Hagelberg On Feb. 9, 2017, 12:47 a.m., David Kantor wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56463/ > ----------------------------------------------------------- > > (Updated Feb. 9, 2017, 12:47 a.m.) > > > Review request for atlas. > > > Bugs: ATLAS-1539 > https://issues.apache.org/jira/browse/ATLAS-1539 > > > Repository: atlas > > > Description > ------- > > Integration tests in projects which use the typesystem test jar (e.g. webapp) > were failing when the tests were invoked in the project directory rather than > the top level project, because properties for user-credentials.properties and > policy store files where using a path relative to the working directory, > which amounted to a hard-coded assumption that the tests would only ever be > run from the top level. > Fix: Attempt to load policy store and user credential files as classloader > resource if configured file path does not exist, in the same manner that the > atlas-application.properties is located. Use test copies of these files in > the typesystem src/test/resources rather than a path relative the top level > maven project. > > > Diffs > ----- > > > authorization/src/main/java/org/apache/atlas/authorize/simple/FileReaderUtil.java > 36db7007a14c2afbda7582409e5dfa474fa9a7b6 > > authorization/src/main/java/org/apache/atlas/authorize/simple/PolicyUtil.java > 4f9255a3dee5ac59dcef95e76eae423445be6bdd > > authorization/src/main/java/org/apache/atlas/authorize/simple/SimpleAtlasAuthorizer.java > d6e785366a5fd4dc60496d37b2639ebafa06d087 > typesystem/src/test/resources/atlas-application.properties > 0e6bc4187ad064820d11da137cee3b8b1857560e > typesystem/src/test/resources/policy-store.txt PRE-CREATION > typesystem/src/test/resources/users-credentials.properties PRE-CREATION > webapp/src/main/java/org/apache/atlas/web/dao/UserDao.java > 254d836e5389780b6d10a8d1a664a96b06ad8d1b > > Diff: https://reviews.apache.org/r/56463/diff/ > > > Testing > ------- > > Ran all unit and integration tests with no regressions. > > > Thanks, > > David Kantor > >
