Skip to content

features/configuration 501#539

Merged
sidke merged 4 commits intomasterfrom
features/configuration-501
Mar 12, 2019
Merged

features/configuration 501#539
sidke merged 4 commits intomasterfrom
features/configuration-501

Conversation

@sidke
Copy link
Copy Markdown
Contributor

@sidke sidke commented Mar 12, 2019

api

introduces a barebones configuration file to be used by rust-fil-proofs with a simple api:

set_config(key, value)

  • set global config property named key to value value

get_config(key) -> value

  • get global config property named key as a result

populate rust-fil-proofs.config.toml with any config options, or override by environment variables to the same name

order of preference is:

ENV > rust-fil-proofs.config.toml

usage:

extern crate config;
use config::get_config;

with

if get_config("MAXIMIZE_CONFIG")? {
  ...
}

default configuration

introduce new configuration options and their default values by extending this const defined in storage-proofs/src/config.rs

const DEFAULT_CONFIG: &[(&str, bool)] = &[
    ("MAXIMIZE_CACHING", false),
];

Copy link
Copy Markdown
Contributor

@porcuquine porcuquine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sold on the proliferation of crates, and the possibility of two distinct toml files is confusing (to me).

I think it would make more sense to put this in storage_proofs, with the defaults supplied in code by whichever consumers of the configuration define settings. Having a place to register configuration settings will also simplify derived configurations (as we've discussed), which will benefit from programmability.

We could do this with a macro inside a registration block in the initialization function, (e.g. def_config!{ MAXIMIZE_CACHING: false }, or whatever), eventually. For now, and maybe forever, simple function calls should be fine, register_config("MAXIMIZE_CACHING", false).

Copy link
Copy Markdown
Contributor

@porcuquine porcuquine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sidke sidke merged commit 1c025c7 into master Mar 12, 2019
@sidke sidke deleted the features/configuration-501 branch March 12, 2019 22:41
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

Successfully merging this pull request may close these issues.

2 participants