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

Fix bug 'Unable to load file' in altsrc #1086

Merged
merged 2 commits into from Mar 30, 2020
Merged

Fix bug 'Unable to load file' in altsrc #1086

merged 2 commits into from Mar 30, 2020

Conversation

akramarenkov
Copy link
Contributor

What type of PR is this?

  • [*] bug

What this PR does / why we need it:

When using altsrc, an 'Unable to load Yaml/TOML/etc file' error occurs during startup of the executable file, which can only be resolved by using the flag used to specify the path to the configuration file.

For example, when you run cli app with this configuration:

startFlags := []cli.Flag{
	&cli.StringFlag{
		Name:  "config",
		Value: "",
		Usage: "path to the configuration file",
	},
	altsrc.NewStringFlag(
		&cli.StringFlag{
			Name:  "listen-addr",
			Value: "127.0.0.1",
			Usage: "listening address",
		}),
}

app.Commands = []*cli.Command{
	{
		Name:   "start",
		Action: startHandler,
		Before: altsrc.InitInputSourceWithContext(startFlags, altsrc.NewYamlSourceFromFlagFunc("config")),
		Flags:  startFlags,
	},
}

In the following cases, an error occurs:

[user@localhost altsrc-fix]$ ./main start
...
Error:  Unable to create input source with context: inner error: 
'Unable to load Yaml file '': inner error: 
'unable to determine how to load from path ''

[user@localhost altsrc-fix]$ ./main start --listen-addr 192.168.0.0
...
Error:  Unable to create input source with context: inner error: 
'Unable to load Yaml file '': inner error: 
'unable to determine how to load from path ''

And in the following cases, an error does not occur:

[user@localhost altsrc-fix]$ ./main start --config config.yml --listen-addr 192.168.0.0
[user@localhost altsrc-fix]$ ./main start --config config.yml

To solve this bug, checks were added whether the flag used to specify the path to the configuration file is set. If it is not specified, MapInputSource with default values is returned.

After applying the changes from this PR, the executable file can be run without the flag used to specify the path to the configuration file:

[user@localhost altsrc-fix]$ ./main start
[user@localhost altsrc-fix]$ ./main start --listen-addr 192.168.0.0
[user@localhost altsrc-fix]$ ./main start --config config.yml
[user@localhost altsrc-fix]$ ./main start --config config.yml --listen-addr 192.168.0.0

Testing

Test code:

package main

import (
	"os"
	"fmt"

	"github.com/urfave/cli/v2"
	"github.com/urfave/cli/v2/altsrc"
)

func main() {
	app := cli.NewApp()

	startFlags := []cli.Flag{
		&cli.StringFlag{
			Name:  "config",
			Value: "",
			Usage: "path to the configuration file",
		},
		altsrc.NewStringFlag(
			&cli.StringFlag{
				Name:  "listen-addr",
				Value: "127.0.0.1",
				Usage: "listening address",
			}),
	}

	app.Commands = []*cli.Command{
		{
			Name:   "start",
			Action: startHandler,
			Before: altsrc.InitInputSourceWithContext(startFlags, altsrc.NewYamlSourceFromFlagFunc("config")),
			Flags:  startFlags,
		},
	}

	err := app.Run(os.Args)
	if err != nil {
		fmt.Println("Error: ", err)
		os.Exit(1)
	}
}

func startHandler(c *cli.Context) error {
	fmt.Println("listen-addr: ", c.String("listen-addr"))
	return nil
}

Release Notes

* Fix 'Unable to load file' error when using altsrc

@akramarenkov akramarenkov requested a review from a team as a code owner March 9, 2020 14:35
Copy link
Member

@coilysiren coilysiren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like ci is broken for your PR, can you investigate that?

@akramarenkov
Copy link
Contributor Author

GitHub CI (more precisely, actions/checkout@v1 - https://github.com/urfave/cli/blob/master/.github/workflows/cli.yml#L36) is trying to checkout the non-existent branch fix-altsrc-nil-source-flag (it is present in my fork), like this:
https://github.com/urfave/cli/pull/1086/checks?check_run_id=498847362#step:4:3
but should do a checkout on pull request like here: https://github.com/urfave/cli/pull/1044/checks#step:4:3

A similar issue in actions/checkout@v1 was discussed here: actions/checkout#23

Unfortunately, I do not know how to solve this problem

@coilysiren
Copy link
Member

looks like CI is fixed now! ✨

@coilysiren coilysiren self-requested a review March 12, 2020 19:02
Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@coilysiren coilysiren merged commit 39f9fbd into urfave:master Mar 30, 2020
@coilysiren
Copy link
Member

✅ merged!

@skandyla
Copy link

skandyla commented Aug 5, 2020

Also hit this bug. It seems Release 2.2.0 does not contain this fix.

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

Successfully merging this pull request may close these issues.

None yet

4 participants