node-convict icon indicating copy to clipboard operation
node-convict copied to clipboard

Are defaults being reapplied when specifying multiple config files in one loadFile?

Open rijnhard opened this issue 9 years ago • 2 comments

So I'm doing some crappy benchmarks. https://github.com/mozilla/node-convict/issues/157

And during the test I discovered some non-obvious behaviour.

when specifying multiple files to loadFile it looks like the defaults were being reapplied.

The easiest way to explain is just to look at the bench example. https://github.com/rijnhard/node-crappy-benchmarks/blob/master/config-parser/convict/test1.js

Settings used

  1. default all to 0
  2. system.json
    • {"a": 1, "b": 3, "c": 5, "d": 7}
  3. config.json
    • {"b": 11, "c": 13}
  4. env (we specify env here because it's not configurable in convict)
    • d=0
    • e=99
  5. CLI override
    • -c 19 -d 23

Results:

- a: 1
- b: 11
- c: 19
- d: 23
- e: 99
- Total = 153

all the variables are simply accumulated, and I would expect the following to be equivalent, but they aren't:

var conf = convict({
    a: {
        format: Number,
        default: 0,
        env: 'a',
        arg: 'a'
    },
    b: {
        format: Number,
        default: 0,
        env: 'b',
        arg: 'b'
    },
    c: {
        format: Number,
        default: 0,
        env: 'c',
        arg: 'c'
    },
    d: {
        format: Number,
        default: 0,
        env: 'd',
        arg: 'd'
    },
    e: {
        format: Number,
        default: 0,
        env: 'e',
        arg: 'e'
    }
});

//1 assuming config.json takes priority. == 152 (incorrect)
conf.loadFile('../config-dir/config.json', '../config-dir/system.json');

//2 == 153 (which is correct)
conf.loadFile('../config-dir/system.json');
conf.loadFile('../config-dir/config.json');

also the docs should probably specify more clearly how defaults and multiple files work (priority) when specifying multiple files in one loadfile.

rijnhard avatar Jun 02 '16 08:06 rijnhard

You have an error in your code :

https://github.com/mozilla/node-convict/blob/be350a7ded016543b3eea16953be1f81c4bf34be/lib/convict.js#L446

loadFile accept only one argument.

Then :

With only `{"b": 11, "c": 13}`, I got 152. Then `//1 [...] == 152 (incorrect)` is correct.
process.env["d"] = "0";
process.env["e"] = "99";

var convict = require('convict');

/******************************************************
 * Helpers
 ******************************************************/
function result(data) {
    process.stdout.write(data + '\n');
    process.exit(0);
}

function add(a, b) {
    return parseInt(a) + parseInt(b);
}

function test() {
    var args = Array.prototype.slice.call(arguments),
        def  = args.shift(); //removes first argument and returns it

    return args.reduce(add, def);
}

/******************************************************
 * Configuration
 *  https://www.npmjs.com/package/convict
 *  in convict the order is predefined and not modifiable
 *      Default value
 *      File (config.loadFile())
 *      Environment variables
 *      Command line arguments
 *      Set and load calls (config.set() and config.load())
 ******************************************************/

var conf = convict({
    a: {
        format: Number,
        default: 0,
        env: 'a',
        arg: 'a'
    },
    b: {
        format: Number,
        default: 0,
        env: 'b',
        arg: 'b'
    },
    c: {
        format: Number,
        default: 0,
        env: 'c',
        arg: 'c'
    },
    d: {
        format: Number,
        default: 0,
        env: 'd',
        arg: 'd'
    },
    e: {
        format: Number,
        default: 0,
        env: 'e',
        arg: 'e'
    }
});

/*
    oddly specifying both in one loadFile breaks.
        conf.loadFile('../config-dir/config.json', '../config-dir/system.json'); == 152, causes 'a' to be 0.
        conf.loadFile('../config-dir/system.json', '../config-dir/config.json'); == 145, so apparently the first param takes priority
    since I speculate it reapplies defaults.
 */

conf.load({"b": 11, "c": 13});

var a = conf.get('a'),
    b = conf.get('b'),
    c = conf.get('c'),
    d = conf.get('d'),
    e = conf.get('e');

result(test(a,b,c,d,e));

This issue should be closed because is not reproductible, is a owner error.

A-312 avatar Dec 07 '19 09:12 A-312

My last message is wrong, I don't quote the good function/lines.

loadFile accept only one argument string or array, not two.

https://github.com/mozilla/node-convict/blob/be350a7ded016543b3eea16953be1f81c4bf34be/lib/convict.js#L607-L609

This issue should be closed because is not reproductible, is an op error.

A-312 avatar Dec 08 '19 23:12 A-312