yacl4j icon indicating copy to clipboard operation
yacl4j copied to clipboard

config.yml not interpreted as YAML file

Open Tzrlk opened this issue 8 years ago • 1 comments

When attempting to load a yaml config with the following code:

val config = ConfigurationBuilder.newBuilder()
		.source().fromFileOnClasspath("config.yml")
		.source().fromSystemProperties()
		.source().fromEnvironmentVariables()
		.build(BlameConfig::class.java)

The following stacktrace is thrown:

Exception in thread "main" java.lang.IllegalStateException: Configuration format not supported: config.yml
	at com.yacl4j.core.source.FileConfigurationSource.selectFileConfigurationSource(FileConfigurationSource.java:60)
	at com.yacl4j.core.source.FileConfigurationSource.fromFileOnClasspath(FileConfigurationSource.java:33)
	at com.yacl4j.core.source.ConfigurationSourceBuilder.fromFileOnClasspath(ConfigurationSourceBuilder.java:20)

Either or both of the following options should resolve this:

  • Add yml to the list of file extensions for YAML parsing
  • Overload the method with an extra parameter specifying how to parse the file. One of:
    • a config format enumeration, e.g. .source().fromFileOnClasspath("config", ConfigFormat.YAML)
    • a parsing class reference, e.g. .source().fromFileOnClasspath("config", YamlConfigParser.class)

The second option would be the most "correct", and the auto-detect algorithm could scan (if it doesn't already) the classpath for parsers and query them to see if they fit the bill, passing the parser class to the more specific method for execution.

Tzrlk avatar Nov 15 '17 00:11 Tzrlk

@Tzrlk thanks for reporting this.

It is really expected considering that the only supported extensions are:

  • .yaml
  • .json
  • .properties

Although I agree with you that the overload option would be flexible enough to handle more use cases, I honestly still don't understand why two extensions (i.e. .yaml and .yml) became so popular for the YAML format when the official documentation specifically recommends only one of them.

Given said, I do remember considering an option similar to what you suggested:

ConfigurationBuilder.newBuilder()
		.source().fromYamlFileOnClasspath("yaml-config.whatever-extension")
                .source().fromJsonFileOnClasspath("json-config.whatever-extension")
                .source().fromPropertiesFileOnClasspath("properties-config.whatever-extension")
                .source().fromFileOnClasspath("config.whatever-extension", new MyFileConfigurationParser());

which is probably ok from a "fluency" point of view and flexibility, less nice to maintain and evolve (also considering the other fromFile and fromFileOnPath variants).

Your option could be a better tradeoff with respect to those aspects:

ConfigurationBuilder.newBuilder()
		.source().fromFileOnClasspath("config.yaml") // auto-detect kicks in
                .source().fromFileOnClasspath("config.json")  // auto-detect kicks in
                .source().fromFileOnClasspath("config.properties")  // auto-detect kicks in
                .source().fromFileOnClasspath("config.whatever-extension", new MyFileConfigurationParser());

fabriziocucci avatar Nov 15 '17 08:11 fabriziocucci