GitHub user vanzin opened a pull request:

    https://github.com/apache/spark/pull/22959

    [SPARK-25876][k8s] Simplify kubernetes configuration types.

    There are a few issues with the current configuration types used in
    the kubernetes backend:
    
    - they use type parameters for role-specific specialization, which makes
      type signatures really noisy throughout the code base.
    
    - they break encapsulation by forcing the code that creates the config
      object to remove the configuration from SparkConf before creating the
      k8s-specific wrapper.
    
    - they don't provide an easy way for tests to have default values for
      fields they do not use.
    
    This change fixes those problems by:
    
    - creating a base config type with role-specific specialization using
      inheritance
    
    - encapsulating the logic of parsing SparkConf into k8s-specific views
      inside the k8s config classes
    
    - providing some helper code for tests to easily override just the part
      of the configs they want.
    
    Most of the change relates to the above, especially cleaning up the
    tests. While doing that, I also made some smaller changes elsewhere:
    
    - removed unnecessary type parameters in KubernetesVolumeSpec
    
    - simplified the error detection logic in KubernetesVolumeUtils; all
      the call sites would just throw the first exception collected by
      that class, since they all called "get" on the "Try" object. Now
      the unnecessary wrapping is gone and the exception is just thrown
      where it occurs.
    
    - removed a lot of unnecessary mocking from tests.
    
    - changed the kerberos-related code so that less logic needs to live
      in the driver builder. In spirit it should be part of the upcoming
      work in this series of cleanups, but it made parts of this change
      simpler.
    
    Tested with existing unit tests and integration tests.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/vanzin/spark SPARK-25876

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/22959.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #22959
    
----
commit ea0f8bce23148a32e9d3e2e5ed763621a25a7330
Author: Marcelo Vanzin <vanzin@...>
Date:   2018-11-02T23:58:53Z

    [SPARK-25876][k8s] Simplify kubernetes configuration types.
    
    There are a few issues with the current configuration types used in
    the kubernetes backend:
    
    - they use type parameters for role-specific specialization, which makes
      type signatures really noisy throughout the code base.
    
    - they break encapsulation by forcing the code that creates the config
      object to remove the configuration from SparkConf before creating the
      k8s-specific wrapper.
    
    - they don't provide an easy way for tests to have default values for
      fields they do not use.
    
    This change fixes those problems by:
    
    - creating a base config type with role-specific specialization using
      inheritance
    
    - encapsulating the logic of parsing SparkConf into k8s-specific views
      inside the k8s config classes
    
    - providing some helper code for tests to easily override just the part
      of the configs they want.
    
    Most of the change relates to the above, especially cleaning up the
    tests. While doing that, I also madke some smaller changes elsewhere:
    
    - removed unnecessary type parameters in KubernetesVolumeSpec
    
    - simplified the error detection logic in KubernetesVolumeUtils; all
      the call sites would just throw the first exception collected by
      that class, since they all called "get" on the "Try" object. Now
      the unnecessary wrapping is gone and the exception is just thrown
      where it occurs.
    
    - removed a lot of unnecessary mocking from tests.
    
    - changed the kerberos-related code so that less logic needs to live
      in the driver builder. In spirit it should be part of the upcoming
      work in this series of cleanups, but it made parts of this change
      simpler.
    
    Tested with existing unit tests and integration tests.

----


---

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

Reply via email to