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

Allow setting an explicit configuration file path when running the agent #15

Open
ema-pe opened this issue Nov 19, 2024 · 3 comments
Open
Labels
dfaas-agent Related to the agent component refactoring

Comments

@ema-pe
Copy link
Collaborator

ema-pe commented Nov 19, 2024

Currently, when the DFaaS agent starts, it reads the configuration file (dfaasagent.env) from a fixed absolute directory called /agent:

// Load configuration
_config, err := config.LoadConfig("/agent")

Then, in LoadConfig of the config package, it reads the dfaasagent.env file:

func LoadConfig(path string) (config Configuration, err error) {
viper.AddConfigPath(path)
viper.SetConfigName("dfaasagent")
viper.SetConfigType("env")
viper.SetEnvPrefix("AGENT")
viper.AllowEmptyEnv(true)
viper.AutomaticEnv()
err = viper.ReadInConfig()
if err != nil {
return
}
err = viper.Unmarshal(&config)
return
}

It would be better to tell the agent where to read the configuration file, using the default path only as a fallback.

@ema-pe ema-pe added refactoring dfaas-agent Related to the agent component labels Nov 19, 2024
@miciav
Copy link
Member

miciav commented Nov 19, 2024

I disagree; having a configuration file in a fixed directory is common practice.
I would work in the direction to use also environment variables for a greater flexibility.

@ema-pe
Copy link
Collaborator Author

ema-pe commented Nov 19, 2024

I disagree; having a configuration file in a fixed directory is common practice. I would work in the direction to use also environment variables for a greater flexibility.

I agree to use environment variables (and the config files that contain them). But I would add a command line option to just specify the path to an additional config file that overwrites the contents of the default one.

Now, if the agent doesn't find the configuration file in the fixed directory, it won't print any warnings or errors, resulting in a useless configuration like the one shown here. This can lead to some unpredictable behavior that is hard to understand. I would change this in two ways:

  1. Add default values (taken from dfaasagent.env here) as a configuration file to be embedded or in config.go using the Viper API,
  2. Print the path of all configuration files read during startup in debug mode to know where and which files the agent is reading.

@aletundo
Copy link
Collaborator

I also think that despite it's a common practice to have a fixed default directory where to load the env file / config file, it's good to have a parameter that can override the default location, which should be used only as default fallback (and the agent should or fail to start if no config file is found, or use all the default values / file with a warning).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dfaas-agent Related to the agent component refactoring
Projects
None yet
Development

No branches or pull requests

3 participants