dsmiley commented on issue #1124: SOLR-14151 :Schema components to be loadable from packages URL: https://github.com/apache/lucene-solr/pull/1124#issuecomment-569474142 > The problem is that SRL is directly used in all APIs I don't consider this a problem. Solr has a lot of plugin abstractions and resources that need loading and at least there is a single abstraction (SRL) that serves this purpose. If there were a bunch of loading abstractions then _that_ may be a problem (IMO), but that is not the case yet thankfully. It may be becoming the case here so I'm paying attention to complexity. > ``` > public String getConfigDir() > > public String getDataDir() > > public Properties getCoreProperties() > > public InputStream openConfig(String name) throws IOException > > public InputStream openSchema(String name) > ``` I don't like those methods being there either! Yay; we agree. There are some more that seem dubious to me too, e.g. `persistConfLocally`; _blech!_ > So the common sense thing to do is to create an interface and hide these methods I think that's one option (the one you are taking). I don't care to say what is or isn't "common sense". A down side to this approach is that the system is now more complex as a whole; the SRL you don't like is still there. I'm curious; do you think these unsightly methods on SRL (that we agree should go away) are really fundamental problems with SRL and get in the way of what we are doing today? > Slapping package loading responsibilities on to SRL is the most dangerous thing to do. We have no separation of responsibilities, and we have a class we are all afraid to touch because it may break something and we nobody knows how it works. Sol already has enough such pieces You are a man of strong opinions. I very much disagree with you concerning your view on SRL. SRL has been loading plugins since before a package management system came along. If it worked with a package manager, that seems to me the same responsibility -- it still loads plugins. The implementation would then change to also work with a package manager but it would still be responsible for loading plugins. Perhaps we actually agree much more than it appears. You are proposing a new abstraction that looks _a lot_ like SRL: SolrClassLoader. It's as if you can't bear to say you like SRL so you create something else in its image but without the dead weight we agree shouldn't be in SRL. Can't we instead work to "fix" SRL and have one abstraction? I very much share your concern of being wary of code that "nobody knows how it works" but I don't view SRL as such. You can count me as someone who can help you or anyone understand SRL should you need help ;-). Now _IndexFetcher_ there's a beast I'm scared of.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
