hadoop-common-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Steve Loughran (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HADOOP-24) make Configuration an interface
Date Tue, 15 Apr 2008 18:21:05 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-24?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12589175#action_12589175

Steve Loughran commented on HADOOP-24:

Looking at this, I'm not sure moving to a formal Interface for Configuration is the right
tactic, not if your goal is to allow for different back-ends. Here's why

-there's a lot of code in Configuration which makes config work easier on top of the underling
read/write a few types of name-value mappings, replicating this is expensive and creates an
ongoing maintenance nightmare. 

-JobConf subclasses Configuration, and there are places where code checks for instanceof JobConf
and/or a new JobConf gets created. If you really wanted to hand configuration off to something
other than XML file datasources, you need to handle JobConf too, which is trickier.

Which means, right now, you don't want to extend Configuration, you want to extend JobConf,
then work out how to sneak in a different factory. 

What may be easier would be an inner ConfSource interface, which supported the raw get/set
operations, possibly typed, possibly simple strings. Then you'd be able to create Configurations
bound to different ConfSource back ends. 

Where there are problems here is in error  handling. Currently its Configuration.loadResource()
that faults on bad XML, by throwing a simple RuntimeException. If you allow different conf
sources, you need to allow for trouble happening later on, when the LDAP or JDBC runtime decides
to throw some exception. Either the ConfSource methods are allowed to throw any Exception,
or there is a need for a standard ConfigurationRuntimeException to throw/wrap trouble.

> make Configuration an interface
> -------------------------------
>                 Key: HADOOP-24
>                 URL: https://issues.apache.org/jira/browse/HADOOP-24
>             Project: Hadoop Core
>          Issue Type: Improvement
>          Components: conf
>    Affects Versions: 0.7.2
>            Reporter: Doug Cutting
>            Assignee: Doug Cutting
>         Attachments: conf-with-factory.patch, configuration.java, configuration.patch,
HADOOP-24-2.patch, HADOOP-24-3.patch, HADOOP-24-4.patch, HADOOP-24.patch, HADOOP-24.patch
> The Configuration class should become an interface, e.g.:
> public interface Configuration {
>   String get(String nam);
>   String set(String name, String value);
>   int getInt(String name);
>   void setInt(String name, int value);
>   float getFloat(String name);
>   void setFloat(String name, float value);
>   //... other utility methods based on get(String) and set(String,String) ...
> }
> An abstract class named ConfigurationBase should be implemented as follows:
> public abstract class ConfigurationBase implements Configuration {
>   abstract public String get(String nam);
>   abstract public String set(String name, String value);
>   public  int getInt(String name) { ... implementation in terms of get(String) ... }
>   public void setInt(String name, int value) {... implementation in terms of set(String,
String) ...}
>   public float getFloat(String name)  { ... implementation in terms of get(String) ...
>   public void setFloat(String name, float value)  {... implementation in terms of set(String,
String) ...}
>   //... other utility methods based on get(String) and set(String,String) ...
> }
> A concrete, default implementation will be provided as follows:
> public class ConfigurationImpl implements Writable extends ConfigurationBase {
>   private Properties properties;
>   // implement abstract methods from ConfigurationBase
>   public String get(String name) { ... implemented in terms of props ...}
>   public String set(String name, String value) { .. implemented in terms of props ...
>   // Writable methods
>   public write(DataOutputStream out);
>   public readFields(DataInputStream in);
>   // permit chaining of configurations
>   public Configuration getDefaults();
>   public void setDefaults(Configuration defaults);
> }
> Only code which creates configurations should need to be updated, so this shouldn't be
a huge change.

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

View raw message