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

Conversation

@akramarenkov
Copy link
Contributor

@akramarenkov akramarenkov commented Mar 9, 2020

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 urfave/cli as a code owner Mar 9, 2020
Copy link
Member

@lynncyrin lynncyrin left a comment

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

@akramarenkov
Copy link
Contributor Author

@akramarenkov akramarenkov commented Mar 11, 2020

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

@lynncyrin
Copy link
Member

@lynncyrin lynncyrin commented Mar 12, 2020

looks like CI is fixed now!

@lynncyrin lynncyrin self-requested a review Mar 12, 2020
Copy link
Member

@saschagrunert saschagrunert left a comment

LGTM

@lynncyrin lynncyrin merged commit 39f9fbd into urfave:master Mar 30, 2020
10 of 12 checks passed
10 of 12 checks passed
ubuntu-latest @ Go 1.12
Details
ubuntu-latest @ Go 1.13
Details
ubuntu-latest @ Go 1.14
Details
macos-latest @ Go 1.12
Details
macos-latest @ Go 1.13
Details
macos-latest @ Go 1.14
Details
windows-latest @ Go 1.12
Details
windows-latest @ Go 1.13
Details
windows-latest @ Go 1.14
Details
test-docs
Details
codecov/patch 61.53% of diff hit (target 73.18%)
Details
codecov/project 73.07% (+-0.12%) compared to d648edd
Details
@lynncyrin
Copy link
Member

@lynncyrin lynncyrin commented Mar 30, 2020

merged!

@skandyla
Copy link

@skandyla 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
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants