lukaskalbertodt / confique Goto Github PK

People might want to use some logic of File (e.g. required) but with a non-built-in file type. Right now they have to do the file content loading themselves. It's not a huge deal, but it could potentially be nicer.

figment can automatically get names of expected environment variables. With the following config:

#[derive(Deserialize, Debug, Clone, Serialize)]
pub struct OpenlibraryConfig {
    pub url: String
}

#[derive(Deserialize, Debug, Clone, Serialize)]
pub struct BookConfig {
    pub openlibrary: OpenlibraryConfig
}

#[derive(Deserialize, Debug, Clone)]
pub struct AppConfig {
    #[serde(default)]
    book: BookConfig
}

let conf: AppConfig = Figment::new().merge(Env::raw().split("_")).extract()?;

The above configuration expects BOOK_OPENLIBRARY_URL, without me having to manually annotate it in the struct. Would be nice if confique supported it as well.

Using env and deserialize_with together fails with the following error(s):

error[E0277]: the trait bound `Url: Deserialize<'_>` is not satisfied
  --> yggdrasil/src/config.rs:3:24
   |
3  | #[derive(Debug, Clone, Config)]
   |                        ^^^^^^ the trait `Deserialize<'_>` is not implemented for `Url`
   |
   = help: the following other types implement trait `Deserialize<'de>`:
             &'a Path
             &'a [u8]
             &'a str
             ()
             (T0, T1)
             (T0, T1, T2)
             (T0, T1, T2, T3)
             (T0, T1, T2, T3, T4)
           and 1350 others
   = note: required because of the requirements on the impl of `Deserialize<'_>` for `std::option::Option<Url>`
note: required by a bound in `confique::internal::from_env`
  --> /home/dav1d/.cargo/registry/src/github.com-1ecc6299db9ec823/confique-0.1.3/src/internal.rs:35:25
   |
35 | pub fn from_env<'de, T: serde::Deserialize<'de>>(key: &str, field: &str) -> Result<T, Error> {
   |                         ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `confique::internal::from_env`
   = note: this error originates in the derive macro `Config` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0277`.
error: could not compile `yggdrasil` due to previous error

e.g.

#[derive(Debug, Clone, Config)]
pub struct Settings {
    #[config(env = "RANCHER_SERVER", deserialize_with = utils::url)]
    pub rancher_server: url::Url,
}

mod utils {
    use serde::Deserializer;

    pub fn url<'de, D>(deserializer: D) -> Result<url::Url, D::Error> where D: Deserializer<'de> {
        todo!()
    }
}

Hi, thanks for this great project!

I'm just getting started with confique, and my toy crate's layout separates the binary from the lib crate

[[bin]]
name = "mybinary"
path = "src/bin/mybinary.rs"

This leads me to expose the config as a public struct: (I think it's fine but comments are welcome)

//! Configuration.

use confique;
use std::{net::IpAddr, path::PathBuf};

/// The main configuration.
#[derive(confique::Config)]     ■ missing documentation for a module
pub struct Conf {     ■■■ missing documentation for a struct field
    /// Port to listen on.
    #[config(env = "PORT", default = 8080)]
    pub port: u16,

    /// Bind address.
    #[config(default = "127.0.0.1")]
    pub address: IpAddr,

    /// Logging configuration.
    #[config(nested)]
    pub log: LogConf,
}

/// Log specific configuration.
#[derive(confique::Config)]     ■ missing documentation for a module
pub struct LogConf {     ■■■ missing documentation for a struct field
    /// Whether or not to print the logs to the console.
    #[config(default = true)]
    pub stdout: bool,

    /// File where to write the logs.
    pub file: Option<PathBuf>,

    /// Ignored modules.
    #[config(default = ["debug"])]
    pub ignored_modules: Vec<String>,
}

By the way, the generated templates are truly great, kudos!

As you can see, I'm using your example code, and I added doc comments wherever I could, but I'm getting missing docs warnings. They are triggered by my directive in lib.rs

#![warn(missing_docs)]

Indeed, cargo expand shows the following

...
    pub mod confique_partial_log_conf {
        use super::*;
        pub struct PartialLogConf {
            pub stdout: std::option::Option<bool>,
            pub file: std::option::Option<PathBuf>,
            pub ignored_modules: std::option::Option<Vec<String>>,
        }
...

Am I missing something? I'd like to keep my missing_docs directive.

Thanks!

The test situation is not too bad right now, but ideally there should be more tests still:

• Compile tests for most use cases
• Tests that META is as expected
• Template formatting tests with different configurations
• Deserialization tests
• Testing that all visibilities work (pub, pub(in path), ...)
• ...

Right now all default values are printed in the config template in one line, e.g. # Default value: { foo: 3, bar: 4 } inline-map syntax. If the default values are complex, that's not very pretty to read. Instead it would be nice to have those into multiple lines, e.g.

# Default value:
#   foo: 3
#   bar: 4

Thats a bit tricky to implement, but there are also open questions: when to use one line and when to use multiple ones? Should library users be able to have control over that? Maybe give users even more control over how each default value is formatted? And how to exactly print: see example above, this maybe looks weird?

It's a project I'm currently planing but haven't really found how I want to do yet, and I stumbled upon this crate looking for a name.

It might be useful to treat nested configuration more like normal values? Making them optional or putting them in lists/maps. But there are lots of open questions and I have to reevaluate whether this requirement still makes sense now that we can have maps and arrays as default values.

As part of implementation #10, I fixed the clippy warnings and run fmt. If the maintainer has no fundamental objections to their comments or specific configuration files, then I would first make a separate PR with these edits by adding automatic checks by github actions

This might improve compile times as the function does not need to be instantiated with a specific config type. The function only ever uses C::META anyway.

It could be useful to load lists from environment vars. But then the big question is: what format? Comma separated? There are too many options (JSON or simply separated by ,; :) and none can be used as a solution for everything. So it needs to be configurable for the library user.

I suppose it is already possible now by using deserialize_with, but that's not particularly convenient.

There are still several things I absolutely want to add. This issue is mostly just a note to myself.

Soon

• format should mention env variables
• Add from attribute to deserialize into another type and then use From or TryFrom
  • Or sth like parse
• Validation
• Rename?
• Forward other derives??? (or maybe not, just derive Debug for Partial)
  • We cannot see other derives, only in special circumstances
  • Thus we probably want #[config(derive_for_partial = Debug)] or sth
  • Maybe a more general attribute forward?

Later

• More format options. Maybe we don't need one Option type per file format?
• Tests
• Make sure error messages when misusing proc macro are good. Also when fields don't implement Deserialize and stuff.
• Arrays as default values
• Deserialize sequences from env via comma separated values?
• config.{toml,yaml} kind of syntax? Mhhh probably not?
• Think about env deserialization again
• Nested Vec<_> and HashMap<_, _> maybe?

(Issue moved from here for better discussion)

@amkartashov wrote:

What about allowing to derive Config for enums? I started using your library in my project and have oneof fields described by enums. I parse it with serde fow now, so it's a mixed approach.

I think empty strings should be the same as not defined, e.g.:

$ COLOR= mycli

Users commonly use an empty string in env vars like this since it's easier than removing them. I think potentially this could be configurable (somehow) like viper does.

It would be useful to make it possible for one field to come from multiple different names. Especially interesting for making backwards-compatible changes. New configs can use the new name while old ones still work. We probably just have to forward an attribute to serde 🤔 Well, the config template should probably mention all aliases as well?

Hi,

I'd like to customize the template output for toml, but because FormatOptions and toml::FormatOptions (same for yaml and json5) are non_exhaustive and only implement the Default trait, it looks like I can't instantiate these structs easily. Right now, the only way I can think of is creating a new trait and implementing it for these structs.

If I'm not missing something, would you consider adding each one a function new allowing the user to pass parameters. I realize that maybe it would lead to a breaking change if you add new fields to these structs. Maybe a builder pattern?

Thanks!

Not sure if it's worth it to be honest. But I can imagine people not using it and it's quite a bit of code that could be disabled...

This can be done in clap for example. Might be nice to express sth like that with attributes and have it checked automatically. However, it would increase the scope of this library considerably and it's straight forward to add such a check in your code afterwards.

Hi, I'm trying to deserialize an enum of options, but it fails with the following error.
`invalid type: string "B", expected enum Strategy'

Minimal Example: https://codesandbox.io/p/sandbox/nostalgic-meadow-jstg4k?file=%2Fsrc%2Fmain.rs%3A7%2C2

Some ideas what users might want to configure:

• Whether the type or the field comment from nested fields is used
• Whether the actual value is commented out, e.g. #foo = or foo =
• What string quotes to use (YAML & JSON), i.e. single or double quoted. Also when to allow bare string literals?
• ...

I asked a bunch of people who are very used to dealing with config files for their opinions. To not lose that information, here it is:

• Defaults in config templates?
  • The commented out value should probably always be the default value.
  • The default should probably also be mentioned explicitly in the comment above.
  • A useful example value should be in the comment, not in the commented-out value
• What file formats are relevant today?
  • TOML, YAML, JSON5
  • INI-like thingies (systemd, freedesktop): basically replaced by TOML, one should use TOML as it's well defined.
  • Used in the wild but usually bad for various reasons:
    • JSON
    • XML
    • ...
• Loading a list of values from an environment variable
  • Widely used separators: :, ;, ,,
  • Or [foo,bar] syntax
  • Or JSON syntax
  • But generally always pretty shit. No good solution.
  • No one syntax works for every value.

I constantly need to give the .env.example file to the devops team and maintain it consistency with in-code structure by hand is inconvenient.

In the current implementation of Confique, the best solution is to give a template from a different format (like toml), where all the env keys are presented, but this does not look like the best solution. If there are no fundamental objections within the project, I will add a dotenv template by myself.

Hey 👋 very excited for this crate. The way it was implemented is very similar to ideas I had for my own crate... which I never started.

One thing I would love to see is support for serialization, but more specifically, the skip_serializing_if field setting. We use this extensively on pretty much every field, because we don't want to write back to our config fields with empty or default values.

For example, here's a current config struct of ours:

#[derive(Clone, Debug, Default, Deserialize, Eq, JsonSchema, PartialEq, Serialize, Validate)]
#[schemars(default)]
#[serde(default, rename_all = "camelCase")]
pub struct ProjectConfig {
    #[serde(skip_serializing_if = "is_default")]
    pub depends_on: Vec<ProjectDependsOn>,

    #[serde(skip_serializing_if = "Option::is_none")]
    pub env: Option<FxHashMap<String, String>>,

    #[serde(skip_serializing_if = "is_default")]
    #[validate(custom = "validate_file_groups")]
    pub file_groups: FileGroups,

    #[serde(
        deserialize_with = "deserialize_language",
        skip_serializing_if = "is_default"
    )]
    pub language: ProjectLanguage,

    #[serde(skip_serializing_if = "Option::is_none")]
    pub platform: Option<PlatformType>,

    #[serde(skip_serializing_if = "Option::is_none")]
    #[validate]
    pub project: Option<ProjectMetadataConfig>,

    #[serde(skip_serializing_if = "is_default")]
    #[validate(custom = "validate_tags")]
    pub tags: Vec<String>,

    #[serde(skip_serializing_if = "is_default")]
    #[validate(custom = "validate_tasks")]
    #[validate]
    pub tasks: BTreeMap<String, TaskConfig>,

    #[serde(skip_serializing_if = "is_default")]
    #[validate]
    pub toolchain: ProjectToolchainConfig,

    #[serde(skip_serializing_if = "is_default")]
    #[serde(rename = "type")]
    pub type_of: ProjectType,

    #[serde(skip_serializing_if = "is_default")]
    #[validate]
    pub workspace: ProjectWorkspaceConfig,

    /// JSON schema URI
    #[serde(rename = "$schema", skip_serializing_if = "is_default")]
    pub schema: String,

    /// Unknown fields
    #[serde(flatten)]
    #[schemars(skip)]
    pub unknown: BTreeMap<String, serde_yaml::Value>,
}

In a perfect confique world, I could see it looking something like this:

#[derive(Config)]
#[config(json_schema = true, typescript = true)]
#[serde(rename_all = "camelCase")]
pub struct ProjectConfig {
    #[config]
    pub depends_on: Vec<ProjectDependsOn>,

    #[config]
    pub env: Option<FxHashMap<String, String>>,

    #[config(validate = "validate_file_groups")]
    pub file_groups: FileGroups,

    #[config]
    #[serde(deserialize_with = "deserialize_language")]
    pub language: ProjectLanguage,

    #[config]
    pub platform: Option<PlatformType>,

    #[config]
    pub project: Option<ProjectMetadataConfig>,

    #[config(validate = "validate_tags")]
    pub tags: Vec<String>,

    #[config(validate = "validate_tasks")]
    pub tasks: BTreeMap<String, TaskConfig>,

    #[config]
    pub toolchain: ProjectToolchainConfig,

    #[config]
    #[serde(rename = "type")]
    pub type_of: ProjectType,

    #[config]
    pub workspace: ProjectWorkspaceConfig,

    /// JSON schema URI
    #[config]
    #[serde(rename = "$schema")]
    pub schema: String,

    /// Unknown fields
    #[serde(flatten)]
    #[schemars(skip)]
    pub unknown: BTreeMap<String, serde_yaml::Value>,
}

First of all, great work. I was wondering if you had any plans to support Hocon in the future. I'd be more than glad to work on this feature myself in case you could use some help.

what are your thoughts about accepting FOO=yes|no as valid entries for true/false? I switched to confique for my app but it broke some users since I was using those before

Hey,
just a quick question:
Is there any way to save the loaded config as file (e.g. yaml file)?

same as #25

but for #![deny(clippy::missing_docs_in_private_items)]

To reproduce:

1. Have a config struct that the lib does not make public
2. Use #![deny(clippy::missing_docs_in_private_items)]

Seems like the doc comment is not passed through in this case?

We cannot see other derives, only in special circumstances. Thus we probably want #[config(derive_for_partial = Debug)] or sth.
Maybe a more general attribute forward? Or just derive Debug and nothing else?

Merging is fine and all, but sometimes you want to just load from the first existing file of a list of file locations. Or other stuff.

A user defined function that is called when loading a config file. Mostly useful to automatically call .validate() on nested fields for example. Or maybe even add built-in attributes like non_empty for strings or something like that. This might increase the scope of this library too much though...

Right now default only allows a handful of different expression types. But sometimes you might want to store the value in a Rust constant or call a function or something like that. Maybe we want to allow that?

The obvious problem here is printing that in the config template. Either the user specifies default_in_template = ... for us to use, or we require that the value implements Serialize and somehow do it like this? Both seems rather meh.

And maybe basically no one wants this feature anyway. Or maybe we shouldn't even allow it as the default value should be expressible in simple expressions: that's whats in the config file in the end, after all.

This library has to do some type inference to find out the exact type of integer and float literals. I think it works totally fine for a majority of real world cases, but we can always do better. However, I don't want to make the type inference algorithm too complex as users can also simply add a type suffix, e.g. 27u8.

• Box<[T]>

Often it's useful to allow users to override some (or all) configuration values via command line. Confique should work nicely for that use case. The currently best way to do it is probably to convert the CLI values to a partial type (manually) and then add it via Builder::preloaded. I would like to investigate whether this can be made more convenient and with less duplicate code.

If this improvement has to be CLI-library specific, I am pretty sure I only want to support clap. I only ever use clap with the derive feature and I think it's the most mature library.

Just to throw some random ideas into this issue, maybe one can annotate config fields with #[config(clap(...))] and if any are annotated this way, we will generate an additional type containing the fields annotated that way that has the derive(clap::*) on it. And has a method to convert it to a partial config type. This extra type can then be flattened into the main clap type. But again, haven't thought about this too deeply yet.