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

Autodetect config format based on extension #1702

Closed

Conversation

danhunsaker
Copy link

@danhunsaker danhunsaker commented Mar 15, 2023

What type of PR is this?

  • feature

What this PR does / why we need it:

  • Allows use of DetectNewSourceFromFlagFunc(flag) to support configs in more than one format for a single application, by autodetecting which format to parse with according to the config file's extension.
  • Allows registration of third-party New*SourceFromFlagFunc() functions to new file extensions for support by the autodetector.
  • Preregisters the three built-in parsers as follows:
    • .yamlNewYamlSourceFromFlagFunc
    • .ymlNewYamlSourceFromFlagFunc
    • .jsonNewJSONSourceFromFlagFunc
    • .tomlNewTomlSourceFromFlagFunc
    • .confNewTomlSourceFromFlagFunc

Testing

  • set up a new CLI project using the following (or similar) configuration:
main.go
func main() {
	app := cli.NewApp()
	app.Name = "dynamic-config"
	app.Version = "0.1.0"
	app.Description = "Showcases how to use the DetectNewSourceFromFlagFunc feature"
	
	app.Authors = []*cli.Author{
		{Name: "Hennik Hunsaker", Email: "hennikhunsaker@gmail.com"},
	}
	
	app.Flags = []cli.Flag{
		altsrc.NewIntFlag(&cli.IntFlag{
			Name:      "number",
			Aliases:   []string{"n"},
			Usage:     "the `answer` to spit out",
			Value:     42,
		}),
		&cli.PathFlag{
			Name:      "config",
			Aliases:   []string{"c"},
			Usage:     "specify the configuration `file` to use",
			Value:     "oa2s.conf",
			TakesFile: true,
		},
	}

	app.AddExtension(altsrc.NewDetectableSourcesAppExtension())

	app.Before = altsrc.InitInputSourceWithContext(app.Flags, altsrc.DetectNewSourceFromFlagFunc("config"))
	
	app.Action = func(c *cli.Context) error {
		fmt.Printf("The answer is %d", c.Int("number"))
		return nil
	}
	
	_ = app.Run(os.Args)
}
  • Create the following config files to test against:
config.yaml
number: 43
config.json
{"number": 44}
config.toml
number = 45
config.conf
number = 46
config.yml
number: 47
  • Try the code using a terminal or console:
go run main.go
go run main.go -- --config config.yaml
go run main.go -- --config config.json
go run main.go -- --config config.toml
go run main.go -- --config config.conf
go run main.go -- --config config.yml
  • Compare the results with the following outputs:
The answer is 42
The answer is 43
The answer is 44
The answer is 45
The answer is 46
The answer is 47

Release Notes

Added support to detect the configuration file format from its extension, rather than predefine it in the code.

@danhunsaker danhunsaker requested a review from a team as a code owner March 15, 2023 00:55
@danhunsaker danhunsaker changed the title Autodetect config format based on extension v2 - Autodetect config format based on extension Mar 15, 2023
@skelouse
Copy link
Contributor

Should add .yml aswell for yaml

altsrc/detect_input_source.go Outdated Show resolved Hide resolved
altsrc/detect_input_source.go Outdated Show resolved Hide resolved
@danhunsaker danhunsaker marked this pull request as draft March 15, 2023 07:56
@danhunsaker
Copy link
Author

Should add .yml aswell for yaml

Done!

@danhunsaker
Copy link
Author

Now to add the new tests.

@danhunsaker danhunsaker force-pushed the feat/altsrc-type-autodetect branch 2 times, most recently from bd44aba to 2588179 Compare March 15, 2023 23:02
@danhunsaker danhunsaker marked this pull request as ready for review March 15, 2023 23:09
@skelouse
Copy link
Contributor

Ahh circular dependencies... Moving the InputSourceContext interface to cli is a backwards compatibility issue. I'm not sure how used it is, maybe someone else can provide some insight. @urfave/cli

I put some time into it, and I'm not sure how it could be done differently. Maybe you have other ideas if the compatibility is an issue @danhunsaker

@danhunsaker danhunsaker force-pushed the feat/altsrc-type-autodetect branch 2 times, most recently from 151e8b4 to 1d1e48a Compare March 18, 2023 21:43
@danhunsaker
Copy link
Author

danhunsaker commented Mar 18, 2023

If Go offered a way to attach objects to other objects without defining an explicit name first...

Though maybe we could add something more generic for myriad extensions to use... 🤔

@danhunsaker danhunsaker force-pushed the feat/altsrc-type-autodetect branch 2 times, most recently from 6c2c5ed to 4439bfa Compare March 18, 2023 23:38
@dearchap
Copy link
Contributor

@danhunsaker v2 is only in maintenance mode right now. No new features/capabilities. Can you do this PR for v3/main ?

- Move things back to `altsrc`
- Add support for extensions to `App`
- Implement `DetectableSourcesAppExtension` to hold `detectableSources`
- Refactor code and tests for the revised setup, including making the
  extension only needed for adding additional handlers
@danhunsaker
Copy link
Author

danhunsaker commented Mar 19, 2023

Sure, once v3 is actually out of alpha and into a real release. I can't use/test it until then.

Also, I don't see support for InputSourceContexts (or their equivalent) on v3 in the first place, meaning I'd have to reimplement all of that as well.

@dearchap
Copy link
Contributor

v3 is the mainline branch. Alpha will be out soon. altsrc has been moved into its own repo for v3 since it is ancillary to cli and not a core.

@danhunsaker
Copy link
Author

There's nothing anywhere pointing it out, so I didn't know there was a separate repository. I'll see what I can do with that.

@meatballhat meatballhat added the area/v2 relates to / is being considered for v2 label Mar 26, 2023
@meatballhat meatballhat added this to the Release 2.x milestone Mar 26, 2023
@meatballhat meatballhat added the kind/feature describes a code enhancement / feature request label Mar 26, 2023
@meatballhat meatballhat changed the title v2 - Autodetect config format based on extension Autodetect config format based on extension Mar 26, 2023
@meatballhat
Copy link
Member

@danhunsaker Thanks for your patience! The transitional state of the ./altsrc ➡️ altsrc move is definitely awkward. Sorry about that! I would love to see this new feature land over there if possible. If you're interested in moving the "altsrc" concepts forward like this, we could really use the help 🙇🏼

@meatballhat meatballhat added the status/waiting-for-response Waiting for response from original requester label May 1, 2023
@bartekpacia
Copy link
Contributor

Closing as per the comments above. v2 is in maintenance mode and we don't accept new features for it.

Thank you for the contribution, and if possible, please submit it to https://github.com/urfave/cli-altsrc

@bartekpacia bartekpacia closed this May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/v2 relates to / is being considered for v2 kind/feature describes a code enhancement / feature request status/waiting-for-response Waiting for response from original requester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants