markrmiller commented on a change in pull request #783:
URL: https://github.com/apache/solr/pull/783#discussion_r839957365
##########
File path:
solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/JDBCStream.java
##########
@@ -275,12 +277,10 @@ public void setStreamContext(StreamContext context) {
this.streamContext = context;
}
- /** Opens the JDBCStream */
- public void open() throws IOException {
-
+ protected Driver getDriver() throws IOException {
try {
if (null != driverClassName) {
- Class.forName(driverClassName);
+ Class.forName(driverClassName, true, getClass().getClassLoader());
Review comment:
Should probably add a comment here making it clear there is intention in
using this classloader and what that intention is.
I'm a little unclear the intention myself. I belive Class.forName(className)
actually calls Class.forName(className, true, this.getClass().getClassLoader()?
Generally you want to avoid both if at all possible. Not only do they
potentially behave differently depending on if called from a static context or
not, but you pretty much always want to strive for getting the context
classloader off the current thread because it will generally at least try and
keep the classpath hiearchy sensible - by using a class classloader, there is
always high potential that you get instantiated by a classloader higher up the
chain than what is used by other application code, and the classes from that
code will then be invisible to you.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
