flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (FLINK-3904) GlobalConfiguration doesn't ensure config has been loaded
Date Mon, 25 Jul 2016 14:53:20 GMT

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

ASF GitHub Bot commented on FLINK-3904:
---------------------------------------

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

    https://github.com/apache/flink/pull/2123#discussion_r72078054
  
    --- Diff: flink-core/src/main/java/org/apache/flink/configuration/GlobalConfiguration.java
---
    @@ -44,159 +35,62 @@
     @Internal
     public final class GlobalConfiguration {
     
    -	/** The log object used for debugging. */
     	private static final Logger LOG = LoggerFactory.getLogger(GlobalConfiguration.class);
     
    -	/** The global configuration object accessible through a singleton pattern. */
    -	private static GlobalConfiguration SINGLETON = null;
    -
    -	/** The internal map holding the key-value pairs the configuration consists of. */
    -	private final Configuration config = new Configuration();
    +	public static final String FLINK_CONF_FILENAME = "flink-conf.yaml";
     
     	// --------------------------------------------------------------------------------------------
    -	
    -	/**
    -	 * Retrieves the singleton object of the global configuration.
    -	 * 
    -	 * @return the global configuration object
    -	 */
    -	private static GlobalConfiguration get() {
    -		// lazy initialization currently only for testibility
    -		synchronized (GlobalConfiguration.class) {
    -			if (SINGLETON == null) {
    -				SINGLETON = new GlobalConfiguration();
    -			}
    -			return SINGLETON;
    -		}
    -	}
     
    -	/**
    -	 * The constructor used to construct the singleton instance of the global configuration.
    -	 */
     	private GlobalConfiguration() {}
     
     	// --------------------------------------------------------------------------------------------
    -	
    -	/**
    -	 * Returns the value associated with the given key as a string.
    -	 * 
    -	 * @param key
    -	 *        the key pointing to the associated value
    -	 * @param defaultValue
    -	 *        the default value which is returned in case there is no value associated with
the given key
    -	 * @return the (default) value associated with the given key
    -	 */
    -	public static String getString(String key, String defaultValue) {
    -		return get().config.getString(key, defaultValue);
    -	}
    -
    -	/**
    -	 * Returns the value associated with the given key as a long integer.
    -	 * 
    -	 * @param key
    -	 *        the key pointing to the associated value
    -	 * @param defaultValue
    -	 *        the default value which is returned in case there is no value associated with
the given key
    -	 * @return the (default) value associated with the given key
    -	 */
    -	public static long getLong(String key, long defaultValue) {
    -		return get().config.getLong(key, defaultValue);
    -	}
     
     	/**
    -	 * Returns the value associated with the given key as an integer.
    -	 * 
    -	 * @param key
    -	 *        the key pointing to the associated value
    -	 * @param defaultValue
    -	 *        the default value which is returned in case there is no value associated with
the given key
    -	 * @return the (default) value associated with the given key
    -	 */
    -	public static int getInteger(String key, int defaultValue) {
    -		return get().config.getInteger(key, defaultValue);
    -	}
    -	
    -	/**
    -	 * Returns the value associated with the given key as a float.
    -	 * 
    -	 * @param key
    -	 *        the key pointing to the associated value
    -	 * @param defaultValue
    -	 *        the default value which is returned in case there is no value associated with
the given key
    -	 * @return the (default) value associated with the given key
    +	 * Loads the global configuration from the environment. Fails if an error occurs during
loading. Returns an
    +	 * empty configuration object if the environment variable is not set. In production
this variable is set but
    +	 * tests and local execution/debugging don't have this environment variable set. That's
why we should fail
    +	 * if it is not set.
    +	 * @return Returns the Configuration
     	 */
    -	public static float getFloat(String key, float defaultValue) {
    -		return get().config.getFloat(key, defaultValue);
    -	}
    -
    -	/**
    -	 * Returns the value associated with the given key as a boolean.
    -	 * 
    -	 * @param key
    -	 *        the key pointing to the associated value
    -	 * @param defaultValue
    -	 *        the default value which is returned in case there is no value associated with
the given key
    -	 * @return the (default) value associated with the given key
    -	 */
    -	public static boolean getBoolean(String key, boolean defaultValue) {
    -		return get().config.getBoolean(key, defaultValue);
    +	public static Configuration loadConfiguration() {
    +		final String configDir = System.getenv(ConfigConstants.ENV_FLINK_CONF_DIR);
    +		if (configDir == null) {
    +			return new Configuration();
    +		}
    +		return loadConfiguration(configDir);
     	}
     
     	/**
     	 * Loads the configuration files from the specified directory.
     	 * <p>
    -	 * XML and YAML are supported as configuration files. If both XML and YAML files exist
in the configuration
    -	 * directory, keys from YAML will overwrite keys from XML.
    +	 * YAML files are supported as configuration files.
     	 * 
     	 * @param configDir
     	 *        the directory which contains the configuration files
     	 */
    -	public static void loadConfiguration(final String configDir) {
    +	public static Configuration loadConfiguration(final String configDir) {
     
     		if (configDir == null) {
    -			LOG.warn("Given configuration directory is null, cannot load configuration");
    -			return;
    +			throw new IllegalConfigurationException("Given configuration directory is null, cannot
load configuration");
    --- End diff --
    
    +1 let's also change the other one


> GlobalConfiguration doesn't ensure config has been loaded
> ---------------------------------------------------------
>
>                 Key: FLINK-3904
>                 URL: https://issues.apache.org/jira/browse/FLINK-3904
>             Project: Flink
>          Issue Type: Improvement
>            Reporter: Maximilian Michels
>            Assignee: Maximilian Michels
>            Priority: Minor
>             Fix For: 1.1.0
>
>
> By default, {{GlobalConfiguration}} returns an empty Configuration. Instead, a call to
{{get()}} should fail if the config hasn't been loaded explicitly.



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

Mime
View raw message