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

Override values from configuration from CLI #81

Closed
Shatur opened this issue Sep 8, 2023 · 11 comments
Closed

Override values from configuration from CLI #81

Shatur opened this issue Sep 8, 2023 · 11 comments

Comments

@Shatur
Copy link

Shatur commented Sep 8, 2023

I trying to write an application that reads config and user can override values from it using CLI:

use clap::Parser;
use figment::{
    providers::{Format, Serialized, Toml},
    Figment,
};
use serde::{Deserialize, Serialize};

#[derive(Parser, Debug, Serialize, Deserialize, Default)]
struct Config {
    #[arg(short, long, required = false)]
    edition: String,
}

fn main() {
    let config: Config = Figment::new()
        .merge(Toml::file("Cargo.toml"))
        .merge(Serialized::defaults(Config::parse()))
        .extract()
        .unwrap();

    println!("{:?}", config.edition);
}

But the problem is that Config::parse() requires to set all values. And If I use arg(..., default_value = "...") it will override values from config.

Config::command().get_matches(); allows me to skip specifying edition in CLI (thanks to required = false), but I can't pass ArgMatches struct to Figment.

@SergioBenitez
Copy link
Owner

But the problem is that Config::parse() requires to set all values. And If I use arg(..., default_value = "...") it will override values from config.

Have you tried using join() instead of merge()?

I'm not really sure what you're trying to achieve exactly. That way you've written it, you're overriding everything from the TOML file with the result of Config::parse(); if you reorder the two lines, you'll get the opposite effect. If you use join(), you'll fill-in holes. You just have to choose what you want and what takes precedence. It's all documented, and some previously opened issues also contain examples that might help your case as well.

@Shatur
Copy link
Author

Shatur commented Sep 26, 2023

Have you tried using join() instead of merge()?

Yes

That way you've written it, you're overriding everything from the TOML file with the result of Config::parse();

Yes, this is what I trying to do. But the problem is that it overrides all values from TOML with CLI. I trying to override only ones that is specified by user.
Config::parse() sets all values for Config. If I make any field as default - this default field will override TOML.

@SergioBenitez
Copy link
Owner

Have you tried using join() instead of merge()?

Yes

Using join for the second invocation would mean you keep values specified in the TOML file even when they're present in the values resulting from Config::parse(). Is this what you're looking for? If this doesn't work as I'm suggesting, then we have a bug to fix. But perhaps you want to override all values as long as they're not the default, even when present in the file?

That way you've written it, you're overriding everything from the TOML file with the result of Config::parse();

Yes, this is what I trying to do. But the problem is that it overrides all values from TOML with CLI. I trying to override only ones that is specified by user.

Config::parse() sets all values for Config. If I make any field as default - this default field will override TOML.

The "trick" is to not serialize default values and to instead use serde's defaults or a default provider on deserialization. Concretely, this means using the skip_serializing_if and default serde field attributes as needed. If the values don't need to be passed in by the user, then you might be better off having two structures: one which is the true config, likely the one in your example, and one which is the CLI config structure with Options and skip_serializing_if = Option::is_none for any optional fields. Then you could replace your Config::parse() line with Cli::parse(), and everything should work as intended.

@Shatur
Copy link
Author

Shatur commented Sep 26, 2023

Using join for the second invocation would mean you keep values specified in the TOML file even when they're present in the values resulting from Config::parse(). Is this what you're looking for?
The "trick" is to not serialize default values and to instead use serde's defaults or a default provider on deserialization. Concretely, this means using the skip_serializing_if and default serde field attributes as needed.

No, I trying to do the opposite - let user override some values from TOML with some values from CLI.
Looks like the only way to achieve it is to have a special provider uses Command from clap under the hood. I mentioned it in the first message.
The use case is quite common. I know many apps that load configuration from file, but user can temporarily override any setting using CLI.

@SergioBenitez
Copy link
Owner

SergioBenitez commented Sep 26, 2023

Using join for the second invocation would mean you keep values specified in the TOML file even when they're present in the values resulting from Config::parse(). Is this what you're looking for?

The "trick" is to not serialize default values and to instead use serde's defaults or a default provider on deserialization. Concretely, this means using the skip_serializing_if and default serde field attributes as needed.

No, I trying to do the opposite - let user override some values from TOML with some values from CLI.

Looks like the only way to achieve it is to have a special provider uses Command from clap under the hood. I mentioned it in the first message.

The use case is quite common. I know many apps that load configuration from file, but user can temporarily override any setting using CLI.

What do you mean? If I'm not mistaken, the mechanism I described above accomplishes what you're after without any special changes. You could also develop a special Provider, of course.

In any case, it looks like we're all set here, so I'm closing this out.

@Shatur
Copy link
Author

Shatur commented Sep 26, 2023

Sorry, I misread you. I thought that you thinking that I talking about CLI -> TOML instead of TOML -> CLI.
Right, this solution will work. But it involves a lot of boilerplate (two structs with the same fields). Is there any chance that we could have a custom provider that accepts clap::Command?

@SergioBenitez
Copy link
Owner

Sorry, I misread you. I thought that you thinking that I talking about CLI -> TOML instead of TOML -> CLI.

Right, this solution will work. But it involves a lot of boilerplate (two structs with the same fields). Is there any chance that we could have a custom provider that accepts clap::Command?

I don't think it makes sense to have library-specific providers here, but you can write your own provider if you'd like! You can even release it on crates.io, and assuming it's nicely documented, we can link to it from our docs.

@Shatur
Copy link
Author

Shatur commented Sep 26, 2023

I don't think it makes sense to have library-specific providers here

Makes sense, but don't you have toml, serde_json, etc? Clap also seems quite popular. I think CLI + one of the config formats is the most common case.

@SergioBenitez
Copy link
Owner

Those are formats, not libraries. We could easily swap out the underlying dependencies for others if needed without any changes to our API. We don't expose serde_json, we expose the JSON format. This would be akin to having a CommandLine provider, not clap.

In any case, I'm not sure what such an integration would look like. clap generates a parser, so besides invoking said parser and serializing (as you're doing here and is easy to do), I'm not sure what else we could do. Maybe there's some functionality in clap I'm not aware. Should you create an implementation that is compelling and generic enough, I'm more than happy to consider it.

@Shatur
Copy link
Author

Shatur commented Sep 26, 2023

This would be akin to having a CommandLine provider, not clap.

Fair point.

(as you're doing here and is easy to do), I'm not sure what else we could do

I wouldn't say that it's easy. The workaround is tricky and involves a lot of boilerplate.

Maybe there's some functionality in clap I'm not aware. Should you create an implementation that is compelling and generic enough, I'm more than happy to consider it.

I will try and let you know.

@Notgnoshi
Copy link

Notgnoshi commented Nov 30, 2023

I will try and let you know.

I was able to put together a proof of concept for this here: https://github.com/Notgnoshi/serdoc

I don't really like having to duplicate the Config struct as NullableConfig, but I'm not sure how else to do it (at least with clap)

Please let us know if you find a better / more generic method!!

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

3 participants