[jira] [Commented] (SQOOP-1604) Base/ Marker class for Config and Configuration

["Jarek Jarcec Cecho (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SQOOP-1604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14194124#comment-14194124 ] 

Jarek Jarcec Cecho commented on SQOOP-1604:
-------------------------------------------

It seems to me that throwing a class cast exception when different class is expected then the one that was required is the right thing to do - otherwise we would end up having very weird runtime errors. In this particular case, having Object or base super class serves pretty much the same purpose, so I don't think that it have any effect on the functionality.

I do see a value in having the configuration system (with the annotation and such) working in generic way as we are not describing how the configuration object hierarchy on the connector side have to look like - it's up to the connector developer to fully drive that. It seems to me that requiring a base class won't provide us additional benefits and will remove this ability, so I currently don't see a point in doing so. Am I missing some functionality that we would get if we would force connector developer to have some super class that they have to always inherit from for each configuration class?

> Base/ Marker class for Config and Configuration
> -----------------------------------------------
>
>                 Key: SQOOP-1604
>                 URL: https://issues.apache.org/jira/browse/SQOOP-1604
>             Project: Sqoop
>          Issue Type: Bug
>    Affects Versions: 1.99.4
>            Reporter: Veena Basavaraj
>            Assignee: Veena Basavaraj
>             Fix For: 2.0.0
>
>
> HDFS and Hbase connector code did not seem to have much functionality to in the initializer and destroyer, so provide a common empty class that connector developers can use if they have nothing to override.
> It is pretty much difficult to have a empty initializer and destroyer classes with the current Initializer api
> I wish our Initializer api was more like the below that enforced the typesafety much harder. 
> {code}
> /**
>  * This allows connector to define initialization work for execution,
>  * for example, context configuration.
>  */
> public abstract class Initializer<L extends LinkConfiguration, J extends JobConfiguration> {
>   /**
>    * Initialize new submission based on given configuration properties. Any
>    * needed temporary values might be saved to context object and they will be
>    * promoted to all other part of the workflow automatically.
>    *
>    * @param context Initializer context object
>    * @param linkConfiguration link configuration object
>    * @param jobConfiguration job configuration object for the FROM and TO
>    *        In case of the FROM initializer this will represent the FROM job configuration
>    *        In case of the TO initializer this will represent the TO job configuration
>    */
>   public abstract void initialize(InitializerContext context, L linkConfiguration,
>       J jobConfiguration);
> {code}
> We could have a base marker classes LinkConfiguration / JobConfiguration, FromJobConfiguration, ToConfiguration instead of the annotations, this way code like this would be more type safe rather than using object that can lead to a lot of class cast exception at runt time
> {code}
>     // link config for the FROM part of the job
>     Object fromLinkConfig = ClassUtils.instantiate(fromConnector.getLinkConfigurationClass());
>     ConfigUtils.fromConfigs(fromConnection.getConnectorLinkConfig().getConfigs(), fromLinkConfig);
>     // link config for the TO part of the job
>     Object toLinkConfig = ClassUtils.instantiate(toConnector.getLinkConfigurationClass());
>     ConfigUtils.fromConfigs(toConnection.getConnectorLinkConfig().getConfigs(), toLinkConfig);
>  public Object getConnectorLinkConfig(Direction type) {
>     switch(type) {
>       case FROM:
>         return fromConnectorLinkConfig;
>       case TO:
>         return toConnectorLinkConfig;
>       default:
>         throw new SqoopException(DirectionError.DIRECTION_0000, "Direction: " + type);
>     }
>   }
>   public void setConnectorLinkConfig(Direction type, Object config) {
>     switch(type) {
>       case FROM:
>         fromConnectorLinkConfig = config;
>         break;
>       case TO:
>         toConnectorLinkConfig = config;
>         break;
>       default:
>         throw new SqoopException(DirectionError.DIRECTION_0000, "Direction: " + type);
>     }
>   }
>   /**
>    * All configuration objects
>    */
>   Object fromConnectorLinkConfig;
>   Object toConnectorLinkConfig;
>   Object fromConfig;
>   Object toConfig;
>   Object d
>  
> {code}
> and instead of generic Class, we could have typed Class
> before
> {code}
>   /**
>    * @return Get link configuration group class
>    */
>   @SuppressWarnings("rawtypes")
>   public abstract Class getLinkConfigurationClass();
> {code}
> after
> {code}
>  /**
>    * @return Get link configuration group class
>    */
>   public abstract LinkConfiguration getLinkConfigurationClass();
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)