Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17342#discussion_r112029478
  
    --- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/internal/SharedState.scala ---
    @@ -146,6 +149,7 @@ private[sql] class SharedState(val sparkContext: 
SparkContext) extends Logging {
     }
     
     object SharedState {
    +  URL.setURLStreamHandlerFactory(new FsUrlStreamHandlerFactory())
    --- End diff --
    
    I'm wondering if it's better to add a try..catch around this:
    
    ```
    scala> URL.setURLStreamHandlerFactory(new FsUrlStreamHandlerFactory())
    
    scala> URL.setURLStreamHandlerFactory(new FsUrlStreamHandlerFactory())
    java.lang.Error: factory already defined
      at java.net.URL.setURLStreamHandlerFactory(URL.java:1112)
      ... 48 elided
    ```
    
    Normally this wouldn't matter, but if someone is messing with class loaders 
(e.g. running Spark embedded in a web app in a servlet container), they may run 
into situations where this code might run twice, or may even fail in the first 
time (if the user's application also installs a stream handler).
    
    So I think it's safer to catch the error and print a warning message here. 
But really optimal would be if the "add jar" code didn't use URL at all for 
this. That's for a future change though.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to