rzo1 commented on code in PR #1833:
URL: https://github.com/apache/stormcrawler/pull/1833#discussion_r3026599209


##########
external/solr/pom.xml:
##########


Review Comment:
   
   
     The shade plugin configuration has two issues that will prevent it from 
achieving the goal of embedding Jetty with no transitive Jetty deps:            
                                  
                                                                                
                                                                                
                               
     1. createDependencyReducedPom must be true                                 
                                                                                
                               
                                                                                
                                                                                
                               
     Currently set to false, which means the published POM still declares 
org.eclipse.jetty:* and org.eclipse.jetty.http2:* as transitive dependencies. 
Consumers will pull in both the shaded 
     Jetty (inside the jar) and the original Jetty jars  defeating the purpose. 
                                                                                
                              
                                                                                
                                                                                
                               
     2. solr-solrj-jetty must be included in the artifactSet                    
                                                                                
                               
                                                                                
                                                                                
                               
     solr-solrj-jetty provides HttpJettySolrClient and 
ConcurrentUpdateJettySolrClient, which reference org.eclipse.jetty classes 
directly. Since it is not in the artifactSet, its classes are
      not pulled into the shaded jar and its bytecode is not rewritten. At 
runtime it will try to load the original (unshaded) org.eclipse.jetty.*  either 
causing ClassNotFoundException or  hitting Storm's conflicting Jetty 12.1.x.    
                                                                                
                                                             
                                                                                
                                                                                
                            
     The two wildcard includes already cover all Jetty artifacts in the 
dependency tree (solr-solrj and solr-solrj-zookeeper have zero Jetty transitive 
deps), so no additional includes are   
     needed beyond solr-solrj-jetty.                                            
                                                                                
                               
                                      
   ```
   --- a/external/solr/pom.xml
   +++ b/external/solr/pom.xml
   @@ -59,7 +59,8 @@
                                <createSourcesJar>true</createSourcesJar>
                                <useBaseVersion>true</useBaseVersion>
   -                            
<createDependencyReducedPom>false</createDependencyReducedPom>
   +                            
<createDependencyReducedPom>true</createDependencyReducedPom>
                                <artifactSet>
                                    <includes>
   +                                    
<include>org.apache.solr:solr-solrj-jetty</include>
                                        <include>org.eclipse.jetty:*</include>
                                        
<include>org.eclipse.jetty.http2:*</include>
   ```



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to