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

Is there support for determining whether an option was set? #94

Closed
atc0005 opened this issue Nov 14, 2019 · 11 comments
Closed

Is there support for determining whether an option was set? #94

atc0005 opened this issue Nov 14, 2019 · 11 comments

Comments

@atc0005
Copy link

atc0005 commented Nov 14, 2019

As a disclaimer, I'm new to Go and may just be overlooking obvious.

Short version

Is it possible to determine whether a user set an environment variable/flag?

From spf13/cobra#23:

Is there a way that I can see if a user set a flag, vs. just used the default?

Desired behavior:

  1. If no config file is specified, use default flag value.
  2. If config file is specified, and the user did not set the flag, use the value in the config file
  3. If config file is specified, but the user did set the flag, use the value specified by the user, even if it's just the default flag value.

I'm trying to avoid the situation where a non-default is set in the config, but the user wants to force the program to use the default.

I have a hacky solution that involves parsing the flags twice using different defaults, is there a better approach?

Details

I recently began using the pelletier/go-toml package to support providing configuration options via a file in addition to the env vars/flags support that this package provides.

I first removed the default struct tag from the config struct since pelletier/go-toml supports that same struct tag name and my hope was that I would learn how to "feather" multiple configuration sources together in order to create a composite configuration struct of user-specified and default settings. I moved the job of applying default settings back to a basic constructor that I'm using to setup a default config object.

I then "merge" in a file config struct, overwriting default values in the destination struct with non-default values from the incoming/source struct. This seemed to make sense during the initial implementation, but after a time I came to realize (even being new) that this was not the ideal approach; using this approach still allowed for invalid values to creep in:

  1. the default constructor gives sane initial values to a new config struct (i.e., "base" struct)
  2. create config structs to hold config file settings and env vars/flag settings
  • now three config structs
  1. the config file introduces a non-default value to its copy of the config struct
  2. the args package is provided a default value via flag and the user assumes that their explicit choice will override settings from the config file
  3. config struct merging takes place which checks for non-default values in the "incoming" struct and copies them into the "base" struct, overwriting any values there
  4. when the env vars/flags config struct is evaluated, the explicit value that happens to be the initial default value is examined, and then discarded
  5. the values specified in the config file end up overriding explicit default values chosen via env vars/command-line

In the end, it looks like I need to check to see if the user specified a value explicitly and use that, likely continuing to apply at least light validation to discovered values prior to assigning it to the config struct.

@alexflint
Copy link
Owner

Thanks for creating this issue @atc0005. I've spent a little time reading what you wrote but I haven't yet really comprehended what the core problem with go-arg is. Would you post a short bit of code showing the basic flow that you're trying to use, and what it is that's not working?

@atc0005
Copy link
Author

atc0005 commented Nov 15, 2019

@alexflint thanks for the response and for looking over my notes.

I don't know that that anything is wrong with go-arg; I'm new enough to Go that I'm having a hard time telling whether the go-arg package provides a way to tell the difference between an explicit value specified by the user (env var or flag) that happens to be the same as a default value for a config setting.

In my case I'm looking to merge several configuration sources with the values applied by the go-arg package against its own config struct to be applied last. Before I took the values that go-arg sets and merge them into a "base" config struct, I want to determine whether the value supplied by go-arg is a default value or one chosen by the user.

Looking around, I found that this issue from another config/flags package is a good match for my situation:

spf13/cobra#23

I'll try to get a small prototype bit of code together to better illustrate and will post back once I have it.

The code I'm working with can be found here:

https://github.com/atc0005/elbow/blob/master/config/config.go#L135

I'm new to Go, so I'm sure it's far from idiomatic.

My main goal is to provide a way to build a unified configuration from several source:

  1. Environment variables (go-arg)
  2. Configuration file (go-toml)
  3. Flags (go-arg)

I'm attempting to match what appears to be a common pattern of having flags overrule all and environment variables with the least precedence.

As I understand it, the go-toml package replaces all existing values, so anything it touches gets wiped out and replaced only with the values it finds. I'd like to support specifying a subset of the total options via config file (e.g., system-wide defaults for all users) and the rest via either environment variables or command-line flags. I'd like for anything specified via command-line flag to override anything else.

The problem is that if the initial default setting is "log in text format", the config file overrides that with JSON, but the one-off application run specifies text via command-line flag. I want the command-line flag to win.

Where I've found others posting similar goals, they were pointed at a method similar to this one that indicates whether a flag was set (or not):

https://godoc.org/github.com/spf13/pflag#FlagSet.Changed

Please let me know if any of this doesn't make sense (it's been a long day and I'm crashing).

@alexflint
Copy link
Owner

alexflint commented Nov 15, 2019

@atc0005 Ah ok, I think I understand now. It makes sense that you want the order to be flags > config > env > defaults, and yes I see how that also makes things tricky because you don't know which args came from defaults vs environment vars vs flags, so it's difficult to merge the struct generated by go-arg with the struct generated by go-toml.

I don't immediately have any great suggestions for how to fix this. One option would be to use a different library to load environment vars, which would let you do something more like:

var args MyArgs
args.SomeField = "some default value"  // and so on for the other defaults
somelib.LoadFromEnvironmentVars(&args)  // or whatever the actual API is
toml.LoadFromFile(&args)  // or whatever the actual API is
arg.MustParse(&args)

Another option would be to use a different argument-parsing library that explicitly supports config files.

Another option after that would be to add config files to go-arg (although I'm not super keen on this because different people tend to want all kinds of different formats for their config files)

Another option would be to add some functions to this library so that you can separately load environment variables and command line flags, in which case you could do something like:

var args MyArgs
args.SomeField = "some default value"  // and so on for the other defaults
arg.MustParseEnv(&args)
toml.LoadFromFile(&args)  // or whatever the actual API is
arg.MustParseCommandLine(&args)

@atc0005
Copy link
Author

atc0005 commented Nov 15, 2019

Ah ok, I think I understand now. It makes sense that you want the order to be flags > config > env > defaults, and yes I see how that also makes things tricky because you don't know which args came from defaults vs environment vars vs flags, so it's difficult to merge the struct generated by go-arg with the struct generated by go-toml

What complicates things with the current collection of packages is that go-toml replaces the struct you point it at, so using a shared/common struct to layer the settings on via toml.Unmarshal (typing from mobile, I think that is the name) doesn't work. In an effort to work around that was I creating three different instances of the struct, each dedicated to its own package. The merging part is where I was (am) having issues as I hadn't found a way to tell if a flag was explicitly set (I am repeating myself now).

Thanks for the tips/direction.

Another option would be to add some functions to this library so that you can separately load environment variables and command line flags

Would you be interested in adding support for determining whether a flag was explicitly set by the user? This could make merging config structs more reliable, but I understand if not.

@atc0005
Copy link
Author

atc0005 commented Nov 15, 2019

Would a workaround like this be likely to blow up in my face?

Have alexflint/go-arg target a separate struct with the majority of fields of either string or []string type. I could then have the default values set to something like PLACEHOLDER and if present, the option hasn't been explicitly set, if not, then run validity checks against the provided (and converted) value before merging into a target config struct.

refs atc0005/elbow#161 (comment)

@alexflint
Copy link
Owner

alexflint commented Nov 15, 2019

Did you know that you can use pointers to determine whether a value was specified or not?

var args struct {
  Test *string
}
arg.MustParse(&args)
if args.Test == nil {
  fmt.Println("--test was not provided")
}

However, if you use default values then this does not work:

var args struct {
  Test *string  `default:"abc"`
}
arg.MustParse(&args)
// args.Test will be non-nil whether or not the user provided this value

@atc0005
Copy link
Author

atc0005 commented Nov 15, 2019

Did you know that you can use pointers to determine whether a value was specified or not?

I knew about pointers having nil as their value (zero-value), but I didn't use pointers in the config struct. I used mostly basic types for the values that are set/used. I didn't even think to use pointers to the value types I want to work with. Thanks so much for the suggestion (and your responsiveness to this GH issue).

Is there anything I should be aware of when replacing non-pointer types in the config struct with pointers? I suspect that I would need to dereference as appropriate vs where I didn't have to before (assumption from a newbie), but are there other concerns?

Thanks again for your help with this.

@alexflint
Copy link
Owner

You're very welcome!

Yes you will just need to dereference where necessary (and check for nil before dereferencing). On the go-arg side of things, the library should handle pointer fields without you needing to do anything special.

@atc0005
Copy link
Author

atc0005 commented Nov 22, 2019

This experience has already taught me so much about pointer handling; I'm glad I embarked in this direction due to what I've learned, but I've grossly underestimated how awkward it would be to convert the current code I have in order to properly handle pointers.

This has been good, but very humbling. It was a useful reminder of how far I still have to go before I can consider myself proficient with Go.

Thanks again for the help.

Please feel free to close this issue when ready.

If it would be useful, I can submit a separate issue as a feature request to optionally toggle environment variable handling for this library/package.

@alexflint
Copy link
Owner

OK good to hear @atc0005. Thanks for your thorough and detailed messages.

@SamuelMarks
Copy link

@atc0005 I got some of this working in my project by hacking together solutions like:

func FieldAndValueWhenNonDefaultValue(configStruct ConfigStruct) map[string]interface{} {
	val := reflect.ValueOf(configStruct)
	fieldToValue := make(map[string]interface{})

	for i := 0; i < val.NumField(); i++ {
		field := val.Type().Field(i)
		defaultTag := field.Tag.Get("default")
		fieldValue := fmt.Sprintf("%v", val.Field(i).Interface())

		if defaultTag != fieldValue && defaultTag != "" {
			fieldToValue[field.Name] = fieldValue
		}
	}

	return fieldToValue
}

(see my trackbacked issue for more detail supporting some of your use-case; not env var but everything else)

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

No branches or pull requests

3 participants