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

Implement a config resolver #209

Merged
merged 1 commit into from
Sep 13, 2017

Conversation

carlpett
Copy link

@carlpett carlpett commented Sep 13, 2017

As discussed in #208 and on gitter: This PR implements a method for populating flags from an user-defined external source, such as a config file. Some properties:

  • This source has lower priority than flags set on the command-line, and envars.
  • Multiple resolvers can be added, and they will be used in the order added, until the first match is found.
  • The lookup is performed using the name of the flag, unless the user sets a custom one.
  • It is possible to disable usage of the resolvers on a per-flag basis.

Things I'm not sure about and would appreciate some discussion around:

  • Currently it is not possible to add a resolver directly on a flag. I don't see that this would be useful, but it wouldn't be hard to add if there is a use-case?
  • I did not check in any implementations for users, but possibly it would be useful to ship some standard ones such as yaml/json file sources? Probably in a separate package in that case?
  • Errors must be handled in the Resolve method. I'm not sure how an error would best be handled if propagated back up the stack? But I guess someone will implement a remote call resolver, which could lead to all sorts of interesting errors, so maybe it would be better to have it after all...

(Also, naming, of course)

@alecthomas
Copy link
Owner

Currently it is not possible to add a resolver directly on a flag. I don't see that this would be useful, but it wouldn't be hard to add if there is a use-case?

I don't think that's necessary.

I did not check in any implementations for users, but possibly it would be useful to ship some standard ones such as yaml/json file sources? Probably in a separate package in that case?

A JSON one would be fine, as it's in the stdlib, but I don't think a YAML one is necessary.

Errors must be handled in the Resolve method. I'm not sure how an error would best be handled if propagated back up the stack? But I guess someone will implement a remote call resolver, which could lead to all sorts of interesting errors, so maybe it would be better to have it after all...

Yes, errors should be handled.

I'm going to merge this, then make a couple of changes (rather than harassing you with even more comments :)). I'll implement the error handling when I do that.

Thanks again for taking the time to implement this!

@alecthomas alecthomas merged commit 76855bf into alecthomas:v3-unstable Sep 13, 2017
@daixiang0
Copy link
Contributor

daixiang0 commented Jan 14, 2019

@alecthomas @carlpett hi, i do not find docs about this, is that missed?

@daixiang0
Copy link
Contributor

@alecthomas @carlpett @Joseph-Irving any update?

@carlpett
Copy link
Author

@daixiang0 As far as I know, this is still only implemented on the v3 branch, so that would be why it is not documented on the v2/master branch.

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

3 participants