[GitHub] [solr] markrmiller commented on a change in pull request #783: SOLR-16131: Fix class loader for jdbcStream

[GitBox <gi...@apache.org>]

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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org