[
https://issues.apache.org/jira/browse/SOLR-243?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12578553#action_12578553
]
Hoss Man commented on SOLR-243:
-------------------------------
John: first off, i want to apologize for the *extremely* belated patch review
... I know i told you i'd try to look at this months ago, but ... yeah ... life
tends to get in the way of patches.
on the whole, things seem pretty good -- although you were still using an
interface instead of an abstract class. Having the main API for a type of
plugin be an abstract class is an important mechanism to help future proof
ourselves against possible additions we want to make the the plugin API in the
future; if it's an interface we can't really change it once it's released
(because we might break people Impls); if it's an abstract class, we can always
provide a default impl for people.
the biggest concern i have about this patch at the moment is that there are no
tests, and no example configs, so it's hard to be sure it's even working at all
:)
here's what i see as the current todo list for this issue...
1) there are some legacy SolrIndexSearcher constructors that need to delegate
to the SolrCore to get indexReaderFactory ... perhaps we should have a helper
method that decides which SolrIndexConfig to use based on the "name" ?
2) we need tests showing custom IndexReaderFactory getting used (even if it's
just a mock IndexReaderFactory thatsets a boolean to show it's being used) ...
this will also serve as a test that config syntax works.
3) need commented out example of using a custom indexReaderFactory in
example/solr/conf/solrconfig.xml
4) sanity check this against SOLR-465, make sure we aren't painting ourselves
into a corner.
5) we should make IndexReaderFactory use the AbstractPluginLoader stuff and
remove the guts of SolrIndexConfig.loadIndexReaderFactory. It looks like we'll
need to add a "single item" version of load to the AbstractPluginLoader to make
that work well.
#5 is something that can be done after this is committed, but 1-4 are pretty
important. If you can update the patch with some configs/tests i'll think
about the legacy SolrIndexSearcher constructors and try to figure out a good
solution for them
> Create a hook to allow custom code to create custom IndexReaders
> ----------------------------------------------------------------
>
> Key: SOLR-243
> URL: https://issues.apache.org/jira/browse/SOLR-243
> Project: Solr
> Issue Type: Improvement
> Components: search
> Affects Versions: 1.3
> Environment: Solr core
> Reporter: John Wang
> Attachments: indexReaderFactory.patch, indexReaderFactory.patch,
> indexReaderFactory.patch, indexReaderFactory.patch, indexReaderFactory.patch
>
>
> I have a customized IndexReader and I want to write a Solr plugin to use my
> derived IndexReader implementation. Currently IndexReader instantiation is
> hard coded to be:
> IndexReader.open(path)
> It would be really useful if this is done thru a plugable factory that can be
> configured, e.g. IndexReaderFactory
> interface IndexReaderFactory{
> IndexReader newReader(String name,String path);
> }
> the default implementation would just return: IndexReader.open(path)
> And in the newSearcher and getSearcher methods in SolrCore class can call the
> current factory implementation to get the IndexReader instance and then build
> the SolrIndexSearcher by passing in the reader.
> It would be really nice to add this improvement soon (This seems to be a
> trivial addition) as our project really depends on this.
> Thanks
> -John
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
