> On Feb. 10, 2017, 8:46 p.m., Jeff Hagelberg wrote: > > authorization/src/main/java/org/apache/atlas/authorize/simple/PolicyUtil.java, > > line 125 > > <https://reviews.apache.org/r/56463/diff/4/?file=1628058#file1628058line125> > > > > Is it worth documenting this new behavior anywhere?
Documented in javadoc of ApplicationProperties.getFileAsInputStream. > On Feb. 10, 2017, 8:46 p.m., Jeff Hagelberg wrote: > > authorization/src/main/java/org/apache/atlas/authorize/simple/PolicyUtil.java, > > line 160 > > <https://reviews.apache.org/r/56463/diff/4/?file=1628058#file1628058line160> > > > > Should we use the Thread's context classloader here? Fixed > On Feb. 10, 2017, 8:46 p.m., Jeff Hagelberg wrote: > > webapp/src/main/java/org/apache/atlas/web/dao/UserDao.java, line 66 > > <https://reviews.apache.org/r/56463/diff/4/?file=1628063#file1628063line66> > > > > 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. Great idea, refactored logic into new utility method ApplicationProperties.getFileAsInputStream() - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56463/#review165171 ----------------------------------------------------------- On Feb. 12, 2017, 5:41 p.m., David Kantor wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56463/ > ----------------------------------------------------------- > > (Updated Feb. 12, 2017, 5:41 p.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 > common/src/main/java/org/apache/atlas/ApplicationProperties.java > 9b1e9cd48a52da1cc388a808a67817e41741209d > 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 > >
