lucene-solr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Hoss Man (JIRA)" <>
Subject [jira] Commented: (SOLR-1449) solrconfig.xml syntax to add classpath elements from outside of instanceDir
Date Fri, 09 Oct 2009 00:42:31 GMT


Hoss Man commented on SOLR-1449:

bq. My current plan is to make the changes as soon as t 1.4 is released. So the refactoring
is going to happen 'now' .

"now" is pre 1.4 ... whatever changes we anticipate making after 1.4 are "later" that was
a big part of my point: we can easily refactor all of this after 1.4 is released, we can even
do it an hour after 1.4 is released, but i was trying to minimize the number of changes needed
to make this work prior to 1.4.

bq. a simpler patch. SolrConfig is agnostic of SolrResourceLoader

How can you even remotely attempt to make that claim? Nothing in your patch does anything
to make SolrConfig agnostic of SolrResourceLoader -- SolrConfig still constructs, maintains
a refrence to, and provides the a public getter for the SolrResourceLoader.

To summarize the main differnces i can see between your patch and mine:
   * you move the FileFiltering out of SolrResourceLoader - i have no opinion about that,
as i said it would be easy to refactor, i don't really care where that logic is
   * you've add a protected getPaths method to SolrConfig which exposes a NamedList of FileFilter
objects -- this feels very hackish to me, not just because the method name is very vague,
and not just because it uses NamedList in an odd way, but because it changes made to the contents
of the directory at runtime would be returned by anyone attempting to use these FileFilters
later, but that won't accurately express the state of the paths when the SolrResourceLoader
(that SolrConfig still has a refrence to) was initialized.  Since it's not certain if this
is really what should hapen if/when people re-use a SolrConfig, this may have to change anyway,
and illustrates why i felt like it was better not to try and get ahead of ourselves now.
   * You've moved the responsibility of modifying the classpath (and completeling the initialization
of SolrResourceLoader) to SolrCore...

...this last change  demonstrates the *exact* bug i explained i was trying to avoid:  An embedded
Solr user who constructs their own SolrConfig object will then have an incomplete SolrResourceLoader
(with a Classpath that hasn't been fully intialized yet) which they might use prior to passing
that SolrConfig object to hte constructor of SolrCore.

This may be an esoteric example, and not likely to cause problems for anyone in practice --
but why risk introducing a bug at all when it's *EASIER* to make it work for _everyone_ by
letting SolrConfig be responsible for initializing it?

Nothing in your patch is going to save work down the road when it comes time to *actually*
decouple SolrConfig from SolrResourceLoader ... all you've done here is make it harder to
use SolrConifg today.  At somepoint, to reuse SolrConfigs, we're going to need to break the
existing public SolrConfig constructors, and force legacy embedded Solr users to change the
way they construct their SolrCore + SolrConfig + SolrResourceLoader, so why not wait until
then to worry about this bullshit so we can fix SolrResourceLoader initialization the _right_
way by making people construct a SolrConfig _before_ constructing a SolrResourceLoader and
passing the jar paths in to the SolrResourceLoader constructor?!


Honestly though, it's a waste of time to keep arguing about any of this now, because it's
really just a moot fucking issue at this point.

I posted this patch almost 3 weeks ago, hoping I could get at least one other person to test
it within a few days, so I could commit it and we could smoke test it on the trunk for at
least a week or two before starting the release process for 1.4.  But at this point only Noble
and Miller have acknowledged reading the patch -- miller hasn't acknowledged running/testing
it, and noble and I can't agree on how it should be implemented.  Grant's goal (last I heard)
was to start the release early next week, and even if we all magically agreed on what the
best patch looked like, and one or two people stepped up and said they tested it, I still
wouldn't feel comfortable commiting a change to existing functionality like this so close
to the release date.

I think we need to postpone this to 1.5.

> solrconfig.xml syntax to add classpath elements from outside of instanceDir
> ---------------------------------------------------------------------------
>                 Key: SOLR-1449
>                 URL:
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Hoss Man
>            Assignee: Hoss Man
>             Fix For: 1.4
>         Attachments: SOLR-1449.patch, SOLR-1449.patch, SOLR-1449.patch, SOLR-1449.patch,
SOLR-1449.patch, SOLR-1449.patch, SOLR-1449.patch, SOLR-1449.patch, SOLR-1449.patch, SOLR-1449.patch
> the idea has been discussed numerous times that it would be nice if there was a way to
configure a core to load plugins from specific jars (or "classes" style directories) by path
 w/o needing to copy them to the "./lib" dir in the instanceDir.
> The current workaround is "symlinks" but that doesn't really help the situation of the
Solr Release artifacts, where we wind up making numerous copies of jars to support multiple
example directories (you can't have reliable symlinks in zip files)

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

View raw message