Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: Automatic extraction based on cargo features #48

Closed
Altair-Bueno opened this issue Jul 9, 2022 · 2 comments
Closed

Comments

@Altair-Bueno
Copy link

Altair-Bueno commented Jul 9, 2022

Motivation

Currently Figment requires a couple of lines of code to get started. This includes the config structure (which is reasonable) as well as setting up the extractors. I suggest an alternative interface to generate the configuration by writing a one liner, based on cargo features

Example

A basic developer writing a binary crate called foo writes the following code to extract the Config struct from a toml and environment (import statements elided)

// main.rs
// Impl Serialize and deserialize
struct Config { /**/ }

let config: Config = Figment::new()
    .merge(Toml::file("Foo.toml"))
    .merge(Env::prefixed("FOO_"))
    .extract()?;
# Cargo.toml
[dependencies]
figment = { version = "*", features = ["toml", "env"] }

If the developer later decides to enable yaml support, the following changes are required

// main.rs
// Impl Serialize and deserialize
struct Config { /**/ }

let config: Config = Figment::new()
    .merge(Toml::file("Foo.toml"))
+   .merge(Yaml::file("Foo.yaml")
+   .merge(Yaml::file("Foo.yml")
    .merge(Env::prefixed("FOO_"))
    .extract()?;
# Cargo.toml
[dependencies]
- figment = { version = "*", features = ["toml", "env"] }
+ figment = { version = "*", features = ["toml", "yaml", "env"] }

Notice how the developer is required to add both .yaml and yml files, because the yaml spec allows for both file extensions. This could be improved using a little more rusty magic.

Proposed api

Provide an auto method on Figment that automatically sets up all Figment extractors based on currently enabled features

// figment.rs, inside the impl block
pub fn auto(name:&str) -> Figment {
   let names = [name, name.to_upper(), name.to_lower(), name.to_upper_first()];
   let mut figment = Figment::new();
   
   for name in names {
      #[cfg(feature = json)
      figment = figment.merge(Json::file(format!("{name}.json")));
      #[cfg(feature = toml)
      figment = figment.merge(Toml::file(format!("{name}.toml")));
      #[cfg(feature = yaml)
      figment = figment.merge(Yaml::file(format!("{name}.yml"))).merge(Yaml::file(format!("{name}.yaml")));
      // ...
   }

   // env is the last one always
   #[cfg(feature = env)
   for name in names {
   figment.merge(Env::prefixed(format!("{}_", name.to_upper()));
   }

   figment
}
// main.rs
// Impl Serialize and deserialize
struct Config { /**/ }

// Still allows for extra customisation, if wanted
let config: Config = Figment::auto("foo").extract()?;
# Cargo.toml
[dependencies]
figment = { version = "*", features = ["toml", "env"] }

Adding yaml support is as easy as editing the Cargo.toml. Just include the yaml feature and it will automatically add all variants without modifying main.rs

# Cargo.toml
[dependencies]
- figment = { version = "*", features = ["toml", "env"] }
+ figment = { version = "*", features = ["toml", "env", "yaml"] }

This could be further enhanced by providing a macro that automatically uses the crate name as the config name, as well as offer some degree of configuration with varargs.

// Use crate name as `name` parameter
let config: Config = figment::auto!().extract()?;
// Use custom name
let config: Config = figment::auto!(name="foo").extract()?;
// Override priority order
let config: Config = figment::auto!(order=[env,toml]).extract()?;

Caveats

  • Priority order: The above example code would use settings defined on json files over yaml.
  • Binary size: Adding multiple name variants could result on a slightly bigger binary.
  • Runtime penalty: String concatenation and loops may result on some sort of performance hit.

Advantages

  • Faster to get started: cargo add, write the scheme and let config: Config = figment::auto()
  • Easier to switch providers
  • Typo-proof: Multiple variants (.yaml, .yml, uppercase, lowercase, title...) reduces chances of debugging typos
  • Opt-in feature: Figment::new() can still be used
  • Extensible: Figment::auto() returns a Figment struct, which can be further chained with more extractors

Final thoughts

  • The environment extractor should always have a higher priority than config files (unless it's overridden by macro)
  • Uppercase, lowercase and title names should all be valid names for config files and environment variables. Debugging silly typos is hard (eg. writing dockerfile instead of Dockerfile)
  • I'm willing to write a pull request that implements this feature after discussing the final api. Note that I'm not familiarised with writing Rust's macros yet, but i have some spare time to learn new things
@SergioBenitez
Copy link
Owner

The idea is appealing, in general, but here are some reasons why we might want to avoid this:

  1. The user may not directly control which features are enabled on figment. The set of features enabled in figment is the transitive closure of figment features enabled by all dependencies. For example, if you depend on figment enabling the toml feature but also depend on foo which depends on figment and enables the yaml feature, your "version" of figment (in reality, they're the same) now also enables toml. This is likely rather surprising. Perhaps worse yet, if foo removes the toml feature, then the feature is also removed from your version. This causes the obvious issue of not knowing which sources you're actually reading configuration information from.
  2. As you state, the precedence and logic rules concerning joining/merging don't have an obvious default. Should a TOML file be preferred over of a JSON or YAML file? Should environment variables merge or join? Should files merge or join?
  3. This would be fairly easy to do outside of figment itself, perhaps a figment-auto library, since none of the ideas depend on figment directly. The library could transitively enable feature on figment so that the user never has to interact with figment directly, even.

In general, it seems that we're trading off a few lines of code, which make logic and intent explicit and clear, for an interface that has no obvious defaults and may not work for many cases. For instance, this interface would not work for Rocket, whose default figment looks like this:

        Figment::from(Config::default())
            .merge(Toml::file(Env::var_or("ROCKET_CONFIG", "Rocket.toml")).nested())
            .merge(Env::prefixed("ROCKET_").ignore(&["PROFILE"]).global())
            .select(Profile::from_env_or("ROCKET_PROFILE", Self::DEFAULT_PROFILE))

I suspect it won't work for most libraries, in fact.

What are your thoughts on writing this in an external library?

@SergioBenitez
Copy link
Owner

I'm closing this out based on the above.

@SergioBenitez SergioBenitez closed this as not planned Won't fix, can't repro, duplicate, stale Sep 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants