> On Oct. 24, 2017, 7:42 p.m., Apoorv Naik wrote: > > graphdb/pom.xml > > Line 37 (original), 38 (patched) > > <https://reviews.apache.org/r/63041/diff/3/?file=1867256#file1867256line38> > > > > graphdb-impls looks like an aggregator pom, do you think it's safe to > > remove ? > > > > can we move the graph profiles from root pom to graphdb pom or there's > > a reason behind putting it in the main pom ? > > Graham Wallis wrote: > Thanks Apoorv. Agreed - I have wondered about this in the past as well. > With the refactoring of the POMs we are maybe in the situation where we could > have the main POM free of any graph profiles, so that it just includes the > 'graphdb' module; and the graphdb module both specifies the underlying graph > modules (as today) and handles the dependencies - instead of having them > separate in graphdb-impls. So we would effectively smash those two POM > together. Then of course there are the actual graphdb specific modules. I > think this would be neater; will try it as soon as possible to check it hangs > together. > > Apoorv Naik wrote: > Sounds good.
Thanks Apoorv, I was thinking about this some more and remembered that my first prototype of the revised POMs tried to move the profiles down out of the root POM; but that didn't work because the graph class is needed at the root POM so that the other child projects know it for testing. Given that the graph profiles only set properties it seemed sensible to set all 4 properties in the root POM, rather than also have profile switching in the graphdb or graphdb-impls POMs. We would either need to make the graph version abstract at the top level (not sure how to do that yet, but happy to discuss) or we need to leave at least some graph profile handling in the root POM. For now I am dropping this issue, but we should discuss. I still think it could be good to collapse graphdb and graphdb-impls, but don't want to hold up this patch to do that. Let's discuss and maybe fix via a separate JIRA. > On Oct. 24, 2017, 7:42 p.m., Apoorv Naik wrote: > > pom.xml > > Lines 646 (patched) > > <https://reviews.apache.org/r/63041/diff/3/?file=1867266#file1867266line647> > > > > I see a comment as well which explicitly says not to use -P profile > > activation for building different graph profiles. Is there a specific > > reason you're using the property -DGRAPH-PROVIDER instead of profile names. > > Graham Wallis wrote: > The reason for using -DGRAPH-PROVIDER is to enforce mutual exclusion of > the graphdb modules. I hunted around for some time but concluded that there > is no good way in maven to enforce mutual exclusion of profiles, other than > to use a system property to select the profile. The uniqueness and > single-valued nature of the system property provides the mutual exclusion. > This helps to avoid the dependency-excludes processing that we had previously. > > Apoorv Naik wrote: > Makes sense. Thanks for the clarification Thanks Apoorv. Dropping issue. - Graham ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63041/#review189078 ----------------------------------------------------------- On Oct. 27, 2017, 3:25 p.m., Graham Wallis wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63041/ > ----------------------------------------------------------- > > (Updated Oct. 27, 2017, 3:25 p.m.) > > > Review request for atlas. > > > Repository: atlas > > > Description > ------- > > ATLAS-1757-v2.patch > This patch contains the changes needed for JanusGraph 0.1.1. > > There are a couple of repository test failures, which may be due to TP3 > changes: > > GraphBackedMetadataRepositoryTest.testConcurrentCalls:180 expected [true] but > found [false] > AtlasRelationshipStoreSoftDeleteV1Test>AtlasRelationshipStoreV1Test.testRelationshipAttributeUpdate_NonComposite_OneToMany:331 > expected [3] but found [4] > > > Diffs > ----- > > distro/pom.xml 9bea00852cb10ddca363ef1e726487394f6947f3 > distro/src/conf/atlas-application.properties > 9e8adca16582e34807ab520106a438a5919df7f1 > docs/src/site/twiki/Configuration.twiki > a5069abd2bf92e1d107f62d4f41832532bc97b89 > graphdb/api/pom.xml 186e7455353c4d073f297868cc884c178d102106 > > graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanGraphQuery.java > 288b325acd5649eecee9b67346fb3aa6ff9603d2 > > graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanQueryFactory.java > ac7ff9e81f658e2d0939a9be2555dd9a18083d30 > > graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/TitanGraphQuery.java > dfdb91b587b812a41a407acae8e794a23ec1c077 > > graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/expr/AndCondition.java > db5093f518ca36fbb4ab9786cc4c47349fa05cd8 > > graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/expr/HasPredicate.java > 0652c41bcfb30b098d8aa08ac96685d8f6cad312 > > graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/expr/InPredicate.java > ca0e8ab53e2e1083bf01b486945a0c3cffeda527 > > graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/expr/OrCondition.java > e7a8a75238cf5df0f94b9ceb7723fd983a8866d9 > > graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/expr/QueryPredicate.java > a80522b56558fbf73177eb1cf6fc34ba09f6d769 > graphdb/graphdb-impls/pom.xml 62a09944f7655098319d477d362c7181f4b373d1 > graphdb/janus/pom.xml PRE-CREATION > graphdb/janus/readme.txt PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusEdge.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusEdgeLabel.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusElement.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraph.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphDatabase.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphIndex.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusIndexQuery.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusObjectFactory.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusPropertyKey.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusVertex.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusVertexQuery.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/GraphDbObjectFactory.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/graphson/AtlasElementPropertyConfig.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/graphson/AtlasGraphSONMode.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/graphson/AtlasGraphSONTokens.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/graphson/AtlasGraphSONUtility.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/query/AtlasJanusGraphQuery.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/query/NativeJanusGraphQuery.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/serializer/BigDecimalSerializer.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/serializer/BigIntegerSerializer.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/serializer/StringListSerializer.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/serializer/TypeCategorySerializer.java > PRE-CREATION > > graphdb/janus/src/main/resources/META-INF/services/javax.script.ScriptEngineFactory > PRE-CREATION > > graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java > PRE-CREATION > > graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusDatabaseTest.java > PRE-CREATION > > graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/GraphQueryTest.java > PRE-CREATION > > graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/JanusGraphProviderTest.java > PRE-CREATION > graphdb/janus/src/test/resources/atlas-application.properties PRE-CREATION > graphdb/pom.xml 179d5c6ad412cecaa5609e73f0c6ba386d28f775 > graphdb/readme.txt PRE-CREATION > graphdb/titan0/pom.xml df89e4fa52581f60e5ba962b114911a1b582f0b0 > > graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/query/NativeTitan0GraphQuery.java > f1f1adbfa1dc53141497fe09a53ef356da48276e > > graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/query/Titan0GraphQuery.java > 1b85ada907b3b90cf5bcc6bf3fd8282fe09c85c1 > graphdb/titan0/src/test/resources/atlas-application.properties > 3058330668424e0b860d0bfe932f78bfb3db8ec1 > graphdb/titan1/pom.xml 146155b73c24dbe10408a5d7071d6c9df02cf514 > > graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/query/NativeTitan1GraphQuery.java > 9293dbd711246cbfb7a7646b4c38d28ffa22a43e > > graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/query/Titan1GraphQuery.java > 826523b52725e1aab5f7dffaa62a848e0af3a408 > graphdb/titan1/src/test/resources/atlas-application.properties > 99fe18a9d177403c8d7b41f2486709f1c6f24c3d > pom.xml 288efc7efd73c973e4e91d9307de1ecbf72d44c5 > repository/pom.xml b7eedde0a30a1716bc9665f2acfa02e169ba183f > > repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java > 40c66ddf01b3a0804effe9d451035a3d51b27c8d > > repository/src/main/java/org/apache/atlas/gremlin/Gremlin3ExpressionFactory.java > 750cd84f7d002a442cb2972230a3b41e0d4a8ed2 > repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java > 639077ddaf02e02799bb990319877f38b3e6b247 > > repository/src/main/java/org/apache/atlas/repository/graph/GraphToTypedInstanceMapper.java > bbacb1467c86c414a7f42267a8051613ff4b2c2d > > repository/src/main/java/org/apache/atlas/repository/impexp/ExportService.java > a88c09e84f6de597612ddc7d108c1556082ccbd1 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java > 9b2731933c5ce65d65ea82046463528b25005c16 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java > 9700917e9d8ca7fdaa7dfae4803072275b97e3e1 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java > 36cd980fbaac45e452dff38193c3f074cc5e95ad > > repository/src/main/java/org/apache/atlas/util/AtlasGremlin2QueryProvider.java > f1014c5a5644b28f20928ad028c9bee95cba09e8 > > repository/src/main/java/org/apache/atlas/util/AtlasGremlin3QueryProvider.java > 98845694ec137b6549412a6de474bd009c68f170 > > repository/src/main/java/org/apache/atlas/util/AtlasGremlinQueryProvider.java > e4898bde1a4e42cd51868afd9ee01416f99e5ea3 > > repository/src/test/java/org/apache/atlas/discovery/GraphBackedDiscoveryServiceTest.java > 236134df72c2868e005fcef626feacb8e50f5d52 > > repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepositoryTest.java > eb779ff88b873ac58a2bd7ab88cb9da875388d4d > > repository/src/test/java/org/apache/atlas/repository/impexp/ZipFileResourceTestUtils.java > e9f0d200db0a42136f656e64b5eee0d7e29aa057 > typesystem/src/test/resources/atlas-application.properties > 65dd9a31fb76482dbb4a9feb972929dd7e641c6f > webapp/pom.xml e59f6dc1f547e19bca2d2fb06bd9120ee6520ea3 > > > Diff: https://reviews.apache.org/r/63041/diff/4/ > > > Testing > ------- > > Build and tested (UT runs) with titan0 and janus providers > > > Thanks, > > Graham Wallis > >
