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

    https://github.com/apache/spark/pull/17723#discussion_r119749407
  
    --- Diff: core/pom.xml ---
    @@ -357,6 +357,34 @@
           <groupId>org.apache.commons</groupId>
           <artifactId>commons-crypto</artifactId>
         </dependency>
    +
    +    <!--
    +     Testing Hive reflection needs hive on the test classpath only.
    +     It doesn't need the spark hive modules, so the -Phive flag is not 
checked.
    +      -->
    +    <dependency>
    +      <groupId>${hive.group}</groupId>
    +      <artifactId>hive-exec</artifactId>
    --- End diff --
    
    So I tried a few things, and the one that got me further was just having 
this:
    
    ```
        <dependency>
          <groupId>${hive.group}</groupId>
          <artifactId>hive-metastore</artifactId>
          <scope>test</scope>
        </dependency>
    ```
    
    And nix the others. Adding the others in test scope caused some weird error 
in sbt, even with all dependencies (we have the dependencies you had problems 
with cached locally).
    
    My comment was going to be to add that, then rewrite the code to use the 
metastore API instead of the `Hive` class from hive-exec... but then I noticed 
that test is not doing much, because there are no metastore servers to talk to. 
It's even there, hardcoded in the test:
    
    ```
      test("obtain tokens For HiveMetastore") {
        ...
        credentials.getAllTokens.size() should be (0)
    ```
    
    All it seems to be doing is making sure the reflection-based code is not 
completely broken. That is something already, though.
    
    So I have two suggestions, in order of preference:
    
    - add the dependencies in "provided" scope, and change the code to use 
actual types and not reflection. Because the classes may not exist at runtime, 
that means having to handle `NoClassDefFoundError` in creative ways.
    
    - keep the reflection code, and remove this test. Or maybe move it to a 
separate module as others have suggested.
    
    I kinda like the first because it's always good to avoid reflection, and 
this is a particularly ugly use of it.


---
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