Pattern for having flags override config file

[carlpett]

I'm converting a viper/cobra app to use Kingpin. That app uses the viper functionality of having configuration from both command line flags and a config file, with flags taking priority.

Are there any recommended ways of implementing this pattern with Kingpin? I've experimented with simply reading the file before the flag parsing and assigning values read from the file to the flag vars, but I worry about this possibly not playing nice with Kingpin features such as required flags/override from envars, etc.

[alecthomas]

Kingpin supports loading flags from a file via @<file>, one per line. This has been removed in v3-unstable as it caused some issues, but the functionality is also available in both versions as ExpandArgsFromFile.

So basically, something like:

args, err := kingpin.ExpandArgsFromFile("foo.conf")
args = append(args, os.Args...)

command, err := kingpin.CommandLine.Parse(args)

If you have an existing configuration format, probably the best option is to map it to args somehow, and get Kingpin to parse it.

Does that achieve what you want?

[alecthomas]

I would like a better story for this exact problem, but I haven't come up with a solution that I really like.

[carlpett]

@alecthomas Aha, interesting.

If you have an existing configuration format, probably the best option is to map it to args somehow, and get Kingpin to parse it.

Viper accepts yaml/json/toml/and probably more, but we're guessing that most people only use yaml anyway, so that's what we're targeting initially. How do you mean I'd map this to args?

[carlpett]

Regarding a user story, as a suggestion for API, would it be possible to do something similar to myFlag.Envar("MYENV")? As a rough sketch:

app := kingpin.New("myapp", "Amazing app").DefaultEnvars().ConfigFile("myapp.yaml") // Maybe separate as OptionalConfigFile and RequiredConfigFile?
myFlag := app.Flag("foo", "Value for the foo").Default("12").Int()
anotherFlag := app.Flag("bar", "Value for the bar").Required().ConfigFile("baz").Int()

If no ConfigFile is given on the flag, expect the name of the flag.

Expected priority would be flag > envar > config file.

One interesting problem would be if it would be possible for the user to give the path to the config file as a flag. It is supported by viper, but I guess they do some multiple pass reading there? Would probably add a fair bit of complexity, so not sure if that is worth it.

[alecthomas]

How do you mean I'd map this to args?

Something like the following, though an issue is the unordered nature of maps, but you get the idea.

func ConfigToFlags(config map[string]interface{}) ([]string, error) {
  flags := []string{}
  for k, v := range config {
    flag := k
    values := []string{}
    switch value := v.(type) {
    case bool:
      if !value {
        flag = "no-" + flag
      }
      flags = append(flags, "--" + flag)
      continue
    case string:
      values = append(values, v)
    case float64:
      values = append(values, fmt.Sprintf("%v", v))
    case []string:
      for _, e := range v {
        values = append(values, e)
      }
    // and so on
    }

    for _, value := range values {
      flags = append(flags, "--" + flag + "=" + value)
    }
  }

  return flags
}

Then to use it:

config := map[string]interface{}
err := yaml.Unmarshal(r, &config) // Or whatever
flags := kingpin.ConfigToFlags(config)

[carlpett]

Ah, I see, thanks for the code! I had some time to test this now. It kinda works, in some cases. The file contains some flags that are only valid on some subcommands, so for other commands it will cause errors.
So I'll probably need to do something more specialized, I guess. Thanks anyway, though! It is a useful approach for some cases.

What did you think about my short proposal above?

[alecthomas]

Ah, I see, thanks for the code! I had some time to test this now. It kinda works, in some cases. The file contains some flags that are only valid on some subcommands, so for other commands it will cause errors.

How would you envisage sub-commands working with a configuration file?

Maybe something like this?

flag = "some value"

[subcommand]
another_flag = "another value"

Then flags under [subcommand] would only be utilised when that subcommand is encountered...

What did you think about my short proposal above?

Interesting idea, but my concern is tying Kingpin to a specific configuration format. INI files? YAML? JSON? All of the above? I think the right solution is to have something like ConfigToFlags(), then apps can load configuration from whatever format they prefer. On the other hand, this is potentially more annoying... so there are pros and cons :\

[carlpett]

How would you envisage sub-commands working with a configuration file?
Maybe something like this?
[...]
Then flags under [subcommand] would only be utilised when that subcommand is encountered...

Yeah, something like that. I think the important part is that the same file can be used for all invocations, rather than having one for each subcommand.

Interesting idea, but my concern is tying Kingpin to a specific configuration format. INI files? YAML? JSON? All of the above? I think the right solution is to have something like ConfigToFlags(), then apps can load configuration from whatever format they prefer. On the other hand, this is potentially more annoying... so there are pros and cons :\

Yeah, there would need to be some sort of adapter here, possibly user-supplied rather than having kingpin trying to support every format under the sun. The user supplies an adapter conforming to some interface including say a Resolve(name string) string function. So given my .ConfigFile("foo") example above, if the flag is not given, and there is no matching envar, Resolve("foo") is called and tries to find the value in the file.
(Note: This was without looking anything at the current code, so possibly it would have to look quite a bit different to match envars etc)

[alecthomas]

That's a pretty good idea actually. In fact, the envar code could potentially be refactored to use this approach too...

[carlpett]

I could give it a shot in a PR, if it sounds like a good way forward?

[alecthomas]

Yeah, that would be great. All development is on the v3-unstable branch at the moment, but I realise that you guys have just switched to v2, so I think v2 is fine. The signature should probably accept a *ParseContext, as well as the flag name.

[carlpett]

So, I dug in a bit, and banged out a working prototype. But there are a few changes necessary for this to work as I laid out above. Mainly around how to handle setting the "file adapter" on the application: the resolve calls are from Clause types, and they do not have a reference to the application. To get the prototype working I just passed this down from Application.setDefaults, but I don't think it is something that could work in reality.
A slightly related problem is that I'm not really sure where to get the ParseContext from to send to Resolve?

I worked on the v3 branch, btw, seemed like the best approach?

[alecthomas]

What's the "file adapter" in this context? I would imagine your config resolver API would look something like the following:

app.AddResolver(MyCustomTOMLResolver("config.toml"))
cmd, err := app.Parse(args)

A slightly related problem is that I'm not really sure where to get the ParseContext from to send to Resolve?

The parser should be calling the resolvers, ideally. Envars are currently applied in .setDefaults(context) which is called by app.Parse(). You should be able to hook the resolvers in there somewhere, and pass the context through.

[carlpett]

"File adapter" was the resolver object, ie MyCustomTOMLResolver in your example.

I'm now attaching the resolvers to the parse context, which was an improvement. And then passing that through to Clause.setDefaults (which previously did not take any parameters). However, this means Clause.SetValue needs to pass the context as well, which requires all the XyzVar functions to have the context available...

Is there any other forum I should be discussing this, by the way, or is this the preferred way?

[alecthomas]

Gitter might be easier!

[carlpett]

I think we can close this :) Thanks!

[daixiang0]

@carlpett hi, for kingpin v2, can i use to get value from config/env/cmd line as below:

	defaultConfig = fmt.Sprintf("%s/%s", home, "app.yaml")
	app        = kingpin.New("app", "myapp.").ConfigFile(defaultConfig).DefaultEnvars()
	config = app.Flag("config", "App config.").Default(defaultConfig).String()

	addr       = app.Flag("addr", "Server address.").Default("https://url").ConfigFile(config).Envar("ADDR").String()
	username   = app.Flag("username", "Username.").Default("").ConfigFile(config).Envar("USERNAME").String()