Builder + Figment

[ryanolson]

I'd be curious what your thoughts would be to support a "builder" being a figment::Provider?

Why would we want to do this? The thought here is that we have some configuration that is locally discoverable by local files or environment variables; however, in our code, we want to both discover those local configuration, but then at the point at which we apply the configuration have some last minute developer override.

The natural way for that developer override is the builder pattern, but in this case, we are not building the entire config, but rather providing some sets of overrides to apply on top of the discovered local configuration.

This would be difficult to do manually and difficult to maintain a forked version of this project; however, with the infrastructure in this project, I think the implementation of a FigmentBuilder would be rather trivial; however, I defer to the authors and their guidance.

An interface might be the creation of a FigmentBuilder for use instead of a Builder.

The resulting object would be a Figment rather than a concrete object.

Take for example

#[derive(FigmentBuilder, Serialize, Deserialize, Debug, Clone)]
#[builder(build_fn(private, name = "build_private"))]
pub struct Config {
    pub path: String,
    pub log_level: String,
    pub runtime: RuntimeConfig,
}

#[derive(FigmentBuilder, Serialize, Deserialize, Debug, Clone)]
pub struct RuntimeConfig {
    #[serde(default)]
    pub max_threads: Option<u64>,

    #[serde(default)]
    pub enable_overlap: Option<bool>,
}

impl Config {

  fn build(&self) -> Result<Config> {
    let builder_figment = self.build_private()?;
    let local_config: Config = Figment::new()
      .merge(Toml::file("/etc/my_app/config.toml"))
      .merge(Env::prefixed("MY_APP_"))
      .merge(builder_figment)
      .extract()?

    Ok(local_config)
  }

What this would do is for any value set by the FigmentBuilder, it would set a "key path" for the global profile, Serialized::global().

With the code above:

let config = Config::figment()
  .path("/path/to/my/data")
  .runtime(RuntimeConfig::figment()
      .enable_overlap(Some(true))
      .build()?)
  .build()?;

The Config::build_private would ultimately produce a figment with two key paths set:

• ("path", "/path/to/my/data")
• ("runtime.enable_overlap", true)

To the authors, I have two questions:

• would you consider supporting this in the project?
• if not, would you consider a discussion on how we could use the core of this project to create a new project which heavily depends on this project and ultimately provides this Figment builder functionality?

[ryanolson]

The ask is a bit different from some previously addressed issues in that we are leveraging the builder still as the primary way to construct the object; however, we are instead constructing a partial which provides overrides and the merging behavior is taken care of by figment.

[TedDriggs]

would you consider supporting this in the project?

Not at this time; derive_builder has widespread-enough usage that I am trying to keep it as lean and focused as possible. Also, in the aftermath of the xz attack, I'm very leery of accepting sizable new features or dependencies that I'm not able to review in-depth personally.

if not, would you consider a discussion on how we could use the core of this project to create a new project which heavily depends on this project and ultimately provides this Figment builder functionality?

Happily, though I don't know how much you need to alter the core to make this happen. Would the following work?

#[derive(Builder, FigmentFromBuilder)]
pub struct Config {
    pub path: String,
    pub log_level: String,
    pub runtime: RuntimeConfig,
}

// Generated builder...
pub struct ConfigBuilder {
    // elided
}

impl ConfigBuilder {
    // setters
}

// Generated by FigmentFromBuilder
impl<'a> From<&'a ConfigBuilder> for Figment {
    fn from(v: &'a ConfigBuilder) -> Self {
        let mut figment = Figment::new();

        // generate this for each field
        if let Some(val) = v.path.cloned() {
            figment.join(("path", val));
        }
    }
}

Would something like that work for your use-case?

Note that there are a number of ways to possibly improve/extend this:

• Use TryFrom instead of From if you still want fallible conversions (though I'm not sure how well those would work with your goal to allow partially-completed builders to turn into Figments.
• Read the #[builder(...)] attributes to pick up on the builder pattern (owned vs mutable vs immutable)
• Read the #[builder(...)] attributes to pick up on field renamings and defaults

[ryanolson]

Thanks @TedDriggs very helpful.

I think the only issue is that within the current builder pattern, the RuntimeConfig is actualized as part of the setter and the RuntimeConfigBuilder is out of scope from the generated FigmentFromBuilder. This allows for partials at the top-level, but any nested structs must be completely actualized.

[TedDriggs]

This allows for partials at the top-level, but any nested structs must be completely actualized.

I'm not understanding this piece.

Referring back to this example from the original post:

let config = Config::figment()
  .path("/path/to/my/data")
  .runtime(RuntimeConfig::figment()
      .enable_overlap(Some(true))
      .build()?)
  .build()?;

Based on how runtime(...) is called, it looks to me like the derived ConfigFigmentBuilder is defined as:

struct ConfigFigmentBuilder {
    path: Option<String>,
    log_level: Option<String>,
    runtime: Option<RuntimeConfig>,
}

impl ConfigFigmentBuilder {
    pub fn path(&mut self, path: String) -> &mut Self {
        self.path = Some(path);
        self
    }

    pub fn log_level(&mut self, log_level: String) -> &mut Self {
        self.log_level = Some(log_level);
        self
    }

    pub fn runtime(&mut self, runtime: RuntimeConfig) -> &mut Self {
        self.runtime = Some(runtime);
        self
    }
}

That requires finishing the RuntimeConfig before calling the setter, which we see in the original example where build()? is called.

What this would do is for any value set by the FigmentBuilder, it would set a "key path" for the global profile, Serialized::global().

Based on this line, I think the original example was trying to show was passing a partial of RuntimeConfig into that setter. In that case, the calling code would've been:

let config = Config::figment()
  .path("/path/to/my/data")
  .runtime(RuntimeConfig::figment()
      .enable_overlap(Some(true)))
  .build()?;

I'm not too familiar with figment, so I could be mistaken here, but it feels like this revised example achieves the goals of:

1. Setters for individual properties on RuntimeConfig, with all the compile-time safety that provides
2. The ability to pass a subset of RuntimeConfig to the parent ConfigFigmentBuilder in a single method call. It's somewhat unclear to me how those partials will turn into a completed RuntimeConfig in ConfigFigmentBuilder::build, but I'm guessing figment already has handling for that.

If my adjustment to the desired calling code is correct, the next task is figuring out what type the ConfigFigmentBuilder::runtime method accepts. If the goal is to get populated fields to store with key paths, then it seems that passing an already-built RuntimeConfig is actually disallowed, because the construction of the RuntimeConfig must be deferred until ConfigFigmentBuilder::build is called. However, it seems odd that I'm not allowed to pass a finished struct of the eventual target type to figment, and am instead forced to hand it over field-by-field.