Flag precedence failure for config file (v1)

[ragecryx]

I have a setup as described in Values from alternate input sources (YAML, TOML, and others)

I try to run the executable with command line arguments, like:

tool dostuff --host my.super.host

config file doesn't exist and I don't provide one and the execution stops instead of relying on the provided arguments. The failure seems to happen on command.go:197 (caused by altsrc/flag.go:57 probably) where there is no YAML file to open thus it prints help and exits.

Is there any workaround for this? Is it solved in v2?

[jszwedko]

Hi @ragecryx ,

I agree, the file shouldn't be required; it just shouldn't be loaded if it doesn't exist (at least as optional behavior).

For now you can work around it by checking for file existence before calling altsrc.NewYamlSourceFromFlagFunc, but I'll mark this as an issue.

PRs welcome for this.

Thanks!

[ragecryx]

@jszwedko I was willing to write a fix but the Before hook seems generic enough that skipping the failure check will break someone else's code. This should be handled on altsrc level, like NewYamlSourceFromFlagFunc succeeding if file was not found or provided?

EDIT:
Also the fix should target v1 or v2 or both?

[jszwedko]

Yeah, I think handling this within altsrc itself seems reasonable.

I'd target v1 for now, it can be merged up into v2.

Thanks for taking a look at this!

[tonglil]

How do you check that the fix exists if you haven't parsed the flags yet?

For example, the file flag is --config, you don't know if the user passed abc.yaml or xyz.toml until you parse the flag for --config. By then, the rest of the flags will have been parsed too.

[jszwedko]

@tonglil since altsrc is implemented as a Before function you can check the parsed flag first (the flags will have been parsed by the time it runs).

Something like:

app.Before = func(c *cli.Context) error {
    if _, err := os.Stat(c.String("load")); os.IsNotExist(err) {
        return ...
    }

   return altsrc.NewYamlSourceFromFlagFunc(flags, c.String("load"))(c)
}

[ragecryx]

I made this fix #713 , I tested it in my project and it works with or without config file but I feel bad having the error not logged anywhere. Any tips about that?

[coilysiren]

Hiya! There's a known issue with the current implementation of the CLI, where altsrc is generally clunky and poorly documented. My current idea is that I'm going to move all of the altsrc functionality into the main package in some way, and that's how all of the "altsrc is weird and different???" issues people are having will get resolved. I'm tracking that work here => #833