> On June 6, 2019, 4:50 p.m., Sridhar K wrote: > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphSolrIndexClient.java > > Line 38 (original), 38 (patched) > > <https://reviews.apache.org/r/70789/diff/1/?file=2147928#file2147928line38> > > > > I liked original name. If any one in community wants implement other > > AtlasGZraphIndexClients, they can add their own.
If/when custom request-handler is available for other index-backends, only place that needs to change is AtlasJanusGraphIndexClient. Hence kept this class name neutral. > On June 6, 2019, 4:50 p.m., Sridhar K wrote: > > repository/src/main/java/org/apache/atlas/discovery/FreeTextSearchProcessor.java > > Line 53 (original) > > <https://reviews.apache.org/r/70789/diff/1/?file=2147930#file2147930line55> > > > > I am not sure why you removed it. Ofcourse, it is an optimization. This check is performed else where in this processor. > On June 6, 2019, 4:50 p.m., Sridhar K wrote: > > repository/src/main/java/org/apache/atlas/discovery/SearchContext.java > > Lines 124 (patched) > > <https://reviews.apache.org/r/70789/diff/1/?file=2147931#file2147931line137> > > > > I prefer to not have so many lines. If funcional separation is needed, > > can we remove one line here? the extra line seen is a review board one. There is actually only one line here. > On June 6, 2019, 4:50 p.m., Sridhar K wrote: > > repository/src/main/java/org/apache/atlas/repository/graph/SolrIndexHelper.java > > Line 109 (original), 113 (patched) > > <https://reviews.apache.org/r/70789/diff/1/?file=2147933#file2147933line114> > > > > I know what you wanted to do here. But, this is user entered value, I > > would prefer it to be throwing an exception early on so that the developers > > can notice it and fix it. Otherwise, they might not realize their mistake > > if eat up the issue silently without throwing exception. This works off of typedef in the store; validations should be performed before typedef is committed into the store. Throwing RuntimeException is generally not a good idea; in startup sequences such exceptions can make Atlas server to abort. Also, it doesn't harm to assume default search-weight here. > On June 6, 2019, 4:50 p.m., Sridhar K wrote: > > repository/src/main/java/org/apache/atlas/repository/patches/FreeTextRequestHandlerPatch.java > > Lines 27 (patched) > > <https://reviews.apache.org/r/70789/diff/1/?file=2147935#file2147935line27> > > > > Can we remove this class initialize the SolrIndexHelper in PatchManager > > instead. I am not sure, if we want this to be created in a patch setting. > > > > I rather have it done explicitly AtlasPatchManager after going through > > all patches in the system. Otherwise, the new patches might be added after > > this patch and we might some def types or patches--may be listerner in > > GraphBackedIndexHelper will cover this scenario. But, I still think that > > this should be initialized by PatchManager after all patches are run as an > > explicit step. This patch needs to be applied only once - when Atlas with this commit is run on an earlier deployment. Else, the request-handler will not be created until a typedef change is performed (like CRUD of entity-typedef, classification-typedef). - Madhan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70789/#review215718 ----------------------------------------------------------- On June 6, 2019, 8:18 p.m., Madhan Neethiraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70789/ > ----------------------------------------------------------- > > (Updated June 6, 2019, 8:18 p.m.) > > > Review request for atlas, Ashutosh Mestry, Aadarsh Jajodia, keval bhatt, > Sridhar K, Le Ma, Nikhil Bonte, Nixon Rodrigues, Saqeeb Shaikh, and Sarath > Subramanian. > > > Bugs: ATLAS-3262 > https://issues.apache.org/jira/browse/ATLAS-3262 > > > Repository: atlas > > > Description > ------- > > - skipped full-text index computation & population when free-text is enabled > - added FreeTextRequestHandlerPatch to create /freetext request handler in > Solr > - fixed free-text search handling of typeName filter > > > Diffs > ----- > > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraph.java > edab08cc0 > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphSolrIndexClient.java > a55fc3689 > intg/src/main/java/org/apache/atlas/ApplicationProperties.java 6d0153f7d > > repository/src/main/java/org/apache/atlas/discovery/ClassificationSearchProcessor.java > 1cd507c3b > > repository/src/main/java/org/apache/atlas/discovery/EntitySearchProcessor.java > c4b5ffb12 > > repository/src/main/java/org/apache/atlas/discovery/FreeTextSearchProcessor.java > b1a84c742 > repository/src/main/java/org/apache/atlas/discovery/SearchContext.java > 50edb9faf > repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java > 2e09beda0 > > repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java > 56655a845 > > repository/src/main/java/org/apache/atlas/repository/graph/SolrIndexHelper.java > 20a517ff1 > > repository/src/main/java/org/apache/atlas/repository/patches/AtlasPatchManager.java > 259f24674 > > repository/src/main/java/org/apache/atlas/repository/patches/FreeTextRequestHandlerPatch.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityChangeNotifier.java > 035b02ce4 > > repository/src/main/java/org/apache/atlas/util/AtlasRepositoryConfiguration.java > 1d296057b > repository/src/main/java/org/apache/atlas/util/SearchPredicateUtil.java > 38de885cf > > > Diff: https://reviews.apache.org/r/70789/diff/2/ > > > Testing > ------- > > - verified that free-text handler is created by the patch for the first time > this enhancement is applied in an existing env > - verified that the free-text search handles typeName filter correctly > - pre-commit tests run: > https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/1176/ > > > Thanks, > > Madhan Neethiraj > >
