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

Precendence order #211

Closed
hokisimi opened this issue Jul 4, 2017 · 7 comments · Fixed by A-312/node-blueconfig#1
Closed

Precendence order #211

hokisimi opened this issue Jul 4, 2017 · 7 comments · Fixed by A-312/node-blueconfig#1

Comments

@hokisimi
Copy link

hokisimi commented Jul 4, 2017

Precendence order

I try to test Windows, but do not work following.

**When merging configuration values from different sources, Convict follows precedence rules. The order, from lowest to highest, is:

1.Default value
2.File (config.loadFile())
3.Environment variables
4.Command line arguments
5.Set and load calls (config.set() and config.load())**

In Mac computer Precendence order is work well, But In Windows computer belows order

1.Default value
2.File (config.loadFile())
3.Set and load calls (config.set() and config.load())
4.Environment variables
5.Command line arguments

Set and load calls < Environment variables

In Windows, Set and load calls is not highes order.

@williansouzagonc
Copy link

I have same issue as you, but i'm using mac.

@nathanph
Copy link

The load function suggests that environment variables and arguments always take precedence. This seems like a sane precedence to me but the documentation currently indicates a different precedence and should be updated to reflect what the code is actually doing. If the precedence is going to change, which I don't think should happen, then there should be a major version bump.

/**
 * Loads and merges a JavaScript object into config
 */
load: function(conf) {
  overlay(conf, this._instance, this._schema);
  // environment and arguments always overrides config files
  importEnvironment(rv);
  importArguments(rv);
  return this;
}

@wcho
Copy link

wcho commented May 13, 2019

I prefer load has same precedence with loadFile. A use case is an encrypted conf file which is decrypted and consumed by convict. It's also ok that loadFile accepts object not only file path.

@A-312
Copy link
Contributor

A-312 commented Aug 16, 2019

@madarche this issue should be closed

This issue/problem is not reproductible on Windows, I tried :

const config = convict({
  port: {
    format: 'port',
    default: 5000,
    arg: 'port',
    env: 'PORT'
  }
})

config.load({
  port: 58
})

I got :

λ SET PORT=52                                       
λ echo "%PORT%"                              
"52"                                         
λ node test.js                                
{ port: 52 }
λ node test.js --port 54                      
{ port: 54 }                                 
λ node test.js                                
{ port: 52 }                                 

@zaverden
Copy link
Contributor

@A-312 your example confirms this issue.

According documentation config.load(..) has maximum priority. So, your example should always print { port: 58 }. But you can see that argument and environment have higher priority.

I believe that it is documentation issue, because this comment shows intention to process arguments and environment with higher priority.

@A-312
Copy link
Contributor

A-312 commented Aug 19, 2019

I read a new time, you have right 😕 But, in some cases we want .load( to be before or after env and arg. Maybe we should add an new argument to avoid a break change.

Or allow .set( to get an object in first parameter :

  • config.set(name, value)
  • config.set(object)
    • object({name1: 'value', name2: 'value'}

Like that:

  1. Default value
  2. File (config.loadFile())
  3. Load calls (config.load())
  4. Environment variables (only used when env property is set in schema; can be overridden using the env option of the convict function)
  5. Command line arguments (only used when arg property is set in schema; can be overridden using the args option of the convict function)
  6. Set calls (config.set())

@zaverden
Copy link
Contributor

@A-312 this logic lives for 8 years already. As I see the only problem here is wrong documentation.

In addition I want to say that config.set() order is not obvious too. In #314 I have added an example how it can be overridden though it's original order is 5.

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 a pull request may close this issue.

6 participants