sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Veena Basavaraj (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (SQOOP-1549) Simplifying the Configuration class concept in Connector api
Date Tue, 07 Oct 2014 00:21:34 GMT

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

Veena Basavaraj edited comment on SQOOP-1549 at 10/7/14 12:21 AM:
------------------------------------------------------------------

[~abec]
#1 is moot at this point, since we all agree to flatten and not have hierarchy So we should
drop it. 

#2 Can you provide code examples? I am not sure I completely understand the proposed design

My points:

1.Less classes ( remove boiler plate, code that does really add value)
2.More Descriptive annotations in the apis. Examples will help, but why complicate when examples
can be simple and achieve the same objective.
3. Be really extensible, so that if we add another type of config ( other than LINK/ JOB)
tommorrow none of the apis or design needs to change, so wrap it in one configuration classes
with dedicated annotation for config types
4. Do not force creating dummy classes or objects when everything can be optional. For instance
there is really no need for LinkConfig in a Cassandra connector


was (Author: vybs):
[~abec]
#1 is moot at this point, since we all agree to flatten and not have hierarchy So we should
drop it. 

#2 Can you provide code examples? I am not sure I completely understand the proposed design

My points:

Less classes ( remove boiler plate, code that does really add value)
More Descriptive in the apis. Examples will help, but why complicate when examples can be
simple and achieve the same objective.
Be extensible, so that if we add another type of config ( other than LINK/ JOB) tommorrow
none of the apis or design needs to change, so wrap it in one configuration classes with dedicated
annotation for config types

> Simplifying the Configuration class concept in Connector api
> ------------------------------------------------------------
>
>                 Key: SQOOP-1549
>                 URL: https://issues.apache.org/jira/browse/SQOOP-1549
>             Project: Sqoop
>          Issue Type: Sub-task
>            Reporter: Veena Basavaraj
>            Assignee: Veena Basavaraj
>
> Here is what happens today ( SQOOP-1367 ) when someone needs to write a connector.
> First they start looking at the connector api and sees that they need to implement configuration
classes.  Well after some thinking they realize, they need 3 classes. Why they wonder? But
they continue on and implement 3 classes. In some cases there is really nothing for Link Configuration,
but they still have to create this dummy class for a Configuration Class and then another
dummy one for config class, which if it were me would find it absurd. 
> Then after creating 3 configuration classes, they need to then create atleast 3 config
classes. Note the use of word atleast.  The api is not at all obvious in telling them that
they infact can create more than 3 config classes. It seems like a hidden feature unless until
someone sees some sample code where there is more than one config class per configuration
class. !!
> The naming "getJobConfigurationClass" tells them nothing. You may say javadoc could explain
it, But I wonder why we need to even support 3 configuration classes and more than 3 config
classes.
> {code}
>   /**
>    * @return Get link configuration class
>    */
>   public abstract Class getLinkConfigurationClass();
>   /**
>    * @return Get job configuration group per direction type or null if not supported
>    */
>   public abstract Class getJobConfigurationClass(Direction jobType);
> {code}
> Here is my proposal ( if at all you want to support groups of configs, they atleast name
the class to "ConfiguratioGroup" 
> Here is how the apis makes it obvious, that this class can contain a group of link configs
> {code}
>   /**
>    * @return Get link configuration group class
>    */
>   public abstract Class getLinkConfigurationGroupClass();
>   /**
>    * @return Get job configuration group class per direction type or null if not supported
>    */
>   public abstract Class getJobConfigurationGroupClass(Direction jobType);
> {code}
> [~abec] seems to need some validation from the group on why it should be called "Group".
I have explained my reasoning for this change in https://reviews.apache.org/r/26295/
> Alternatively I think the current design/ implementation to support config parameters
grouping is overkill ( over designed) 
> I prefer simple apis, less things for a developer to code and intuitive names to everything
they represent
> 1.  Remove the ConfigList and support grouping of configs by the "group" attribute on
inputs
> 2.  Have one configuration class annotation  that will mandate 3 classes with specific
annotations attributes on it FromConfig, ToConfig and LinkConfig to be filled. 
> So having one class, gives a complete picture of all configs this connector uses/ provides.
 There is one resource bundle we require, so it maps to one configuration class as well. 
> In code this is how it will look
> {code}
>  */
> @ConfigurationGroupClass
> public class HdfsCongifuration {
>   @LinkConfig public LinkConfig linkConfig;
>   @FromConfig public FromJobConfig fromJobConfig;
>   @ToConfig public ToJobConfig toJobConfig;
>   
>  ....
> }
> {code}
> or
> {code}
> @ConfigurationGroupClass(link="LinkConfig.class", from="fromConfig.class", to="toConfig.class")
> public class HdfsCongifuration {
>  ....
> }
> {code}
> {code}
>  */
> @ConfigClass(validators = {@Validator(ToJobConfig.ConfigValidator.class)})
> public class ToJobConfig {
>   @Input(size = 50, group="foo")   public String schemaName;
>   @Input(size = 2000, group="bar") public String tableName;
>   @Input(size = 50)   public String sql;
>   @Input(size = 50)   public String columns;
>   @Input(size = 2000) public String stageTableName;
>   @Input              public Boolean clearStageTable;
>   
> }
> {code}
>  



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

Mime
View raw message