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

V2: Making altsrc flag optional in Before: #793

Closed
johnwyles opened this issue Jan 18, 2019 · 3 comments
Closed

V2: Making altsrc flag optional in Before: #793

johnwyles opened this issue Jan 18, 2019 · 3 comments

Comments

@johnwyles
Copy link

johnwyles commented Jan 18, 2019

Is there a way we can get an altsrc wrapper function to accomplish the following? Given that all of these features (env variables, flags, and config files) all override each other you do not want to mandate a configuration file be specified and instead make it OPTIONAL that it is specified:

In test.go:

package main

import (
	"fmt"
	"os"

	cli "gopkg.in/urfave/cli.v2"
	"gopkg.in/urfave/cli.v2/altsrc"
)

func main() {
	flags := []cli.Flag{
		altsrc.NewStringFlag(
			&cli.StringFlag{
				Name:  "foo",
				Value: "BAR",
			},
		),
		&cli.StringFlag{
			Name: "load",
		},
	}

	app := &cli.App{

		Action: func(c *cli.Context) error {
			fmt.Printf("load: %s\n", c.String("load"))
			fmt.Printf("foo: %s\n", c.String("foo"))
			return nil
		},

		// This is seen all over and works but REQUIRES you provide "--load" and a TOML file
		// Before: altsrc.InitInputSourceWithContext(flags, altsrc.NewTomlSourceFromFlagFunc("load")),

		// Instead something like this would be desired:
		Before: altsrc.InitInputSourceWithContext(
			flags,
			func(context *cli.Context) (altsrc.InputSourceContext, error) {
				if context.IsSet("load") {
					return altsrc.NewTomlSourceFromFile(context.String("load"))
				}

				return &altsrc.MapInputSource{}, nil
			},
		),

		Flags: flags,
	}

	if err := app.Run(os.Args); err != nil {
		fmt.Errorf("An error occurred: %#v", err)
		os.Exit(1)
	}
}

In test.toml:

foo = "FOO"

And the above test.go will work as expected:

# Works as expected:
$ go run test.go
load:
foo: BAR

# Works as expected:
$ go run --foo OOF
load:
foo: OOF

# Works as expected:
$ go run test.go --load test.toml --foo OOF
load: test.toml
foo: OOF

# Works as expected:
$ go run test.go --load test.toml
load: test.toml
foo: FOO
@johnwyles
Copy link
Author

johnwyles commented Jan 20, 2019

EDIT: I have submitted a PR to address the specific issue originally described below for V1 (#802) and V2 (#803).

I've actually run further into problems in that I would like to do more initialization in Before: after the configuration is loaded, but still before additional commands are loaded, so this is something I think that could be useful. Editing the Before: would be something like the following in pseudocode:

		Before: altsrc.InitInputSourceWithContext(
			flags,
			func(context *cli.Context) (altsrc.InputSourceContext, error) {
				if context.IsSet("load") {
					// Fictitious function to load the configuration merging with Flags and ENV variables
					altsrc.LoadTomlSourceFromFile(context.String("load"))
				}

				switch context.String("log-level") {
				case "debug":
					zerolog.SetGlobalLevel(zerolog.DebugLevel)
				default:
					zerolog.SetGlobalLevel(zerolog.InfoLevel)
				}

				// Make a connect all sub-commands will use
				Queue, err := queue.RabbitMQ(context.String("aqmp-url"))
				if err != nil {
					return
				}

				return &altsrc.MapInputSource{}, nil
			},
		),

@johnwyles
Copy link
Author

johnwyles commented Feb 3, 2019

Note this issue may be resolved in #800 and in PR #802 (v1) and #803 (v2) by allowing Before to be hijacked for altsrc and adding Prepare for other work we would like to perform.

@coilysiren
Copy link
Member

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

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

2 participants