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

Add bind attribute support #11

Open
DCjanus opened this issue May 20, 2022 · 2 comments
Open

Add bind attribute support #11

DCjanus opened this issue May 20, 2022 · 2 comments
Labels
enhancement New feature or request

Comments

@DCjanus
Copy link

DCjanus commented May 20, 2022

Example:

#[derive(Debug, Deserialize)]
pub struct Config {
    pub application: Application,
    // other fields
}

#[derive(Debug, Deserialize)]
pub struct Application {
    pub database: Database,
}

#[derive(Debug, Deserialize)]
pub struct Database {
    #[serfig(bind = "DATABASE_URL")]
    pub url: String,
}

When I was using sqlx, to work with sqlx-cli, command line to manage database migrations, I have to provide database connection options to it, which means I have to put database URL in command line args or environment variable, mostly I'd like to put this in file in name .env.

But in my application, the path to the field may not be database.url, I hope to bind the field in specific path.

@Xuanwo
Copy link
Owner

Xuanwo commented May 21, 2022

Nice feature request! I'm thinking about adding similar features too.

However, this feature seems not trivial.

serfig rely on serde to do deserialization. Inside serfig, we will:

  • Deserialize all values into Self
  • Serialize into serde_bridge::Value
  • Merge all the values
  • Deserialize the final value into Self

So there is small space for us to bind something. I have two ideas so far.

Wrappers for different source

In databend, to make our MetaConfig compatible, I'm adding wrappers to load values from different sources in refactor(meta/config): Adapt RFC Config Backward Compatibility

pub fn load() -> MetaResult<Self> {
    let arg_conf = Self::parse();

    let mut builder: serfig::Builder<Self> = serfig::Builder::default();

    // Load from the config file first.
    {
        let config_file = if !arg_conf.config_file.is_empty() {
            arg_conf.config_file.clone()
        } else if let Ok(path) = env::var("METASRV_CONFIG_FILE") {
            path
        } else {
            "".to_string()
        };

        builder = builder.collect(from_file(Toml, &config_file));
    }

    // Then, load from env.
    let cfg_via_env: ConfigViaEnv = serfig::Builder::default()
        .collect(from_env())
        .build()
        .map_err(|e| MetaError::InvalidConfig(e.to_string()))?;
    builder = builder.collect(from_self(cfg_via_env.into()));

    // Finally, load from args.
    builder = builder.collect(from_self(arg_conf));

    builder
        .build()
        .map_err(|e| MetaError::InvalidConfig(e.to_string()))
}

This way needs a lot of From/Into and is not suitable for a simple DATABASE_URL.

Add bind support in serde-env

Maybe we can bind it in serde-env instead?

It's similar to serde's alias but only affects serde-env and will not change other serde formats.

#[derive(Debug, Deserialize)]
pub struct Config {
    pub application: Application,
    // other fields
}

#[derive(Debug, Deserialize)]
pub struct Application {
    pub database: Database,
}

#[derive(Debug, Deserialize)]
pub struct Database {
    #[serde_env(bind = "database_url")]
    pub url: String,
}

This way seems better, but we still need some investigation on how to implement it.


What do you think?

@DCjanus
Copy link
Author

DCjanus commented May 21, 2022

It's hard to impl than I thought, I've no idea yet.

@Xuanwo Xuanwo added the enhancement New feature or request label May 21, 2022
@Xuanwo Xuanwo moved this to 📋 Backlog in Xuanwo's Work May 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
No open projects
Status: 📋 Backlog
Development

No branches or pull requests

2 participants