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

making Config configurable programmatically #55

Merged
merged 1 commit into from
Dec 18, 2017

Conversation

dbarrosop
Copy link
Contributor

@dbarrosop dbarrosop commented Dec 17, 2017

The idea is that I want to be able to do:

    brigade = Brigade(
        inventory=SimpleInventory("../hosts.yaml", "../groups.yaml"),
        dry_run=True,
        config=Config(raise_on_error=False),
    ) 

@ogenstad
Copy link
Collaborator

I like the idea of this. However, one potential issue I see is that people can set their own config keys in this way. That might cause a problem if we introduce that key as a "real" config value in a later release.

This could be handled by only allowing keys that exist in CONF.

Or perhaps allowing users to set their own config keys in this way by using a prefix like _ or similar. That way we could also load "private" keys if someone sent in a yaml files containing those to the config.

@dbarrosop
Copy link
Contributor Author

However, one potential issue I see is that people can set their own config keys in this way. That might cause a problem if we introduce that key as a "real" config value in a later release.

Well, that's true for any class. They could do the same in any other class.

Or perhaps allowing users to set their own config keys in this way by using a prefix like _ or similar

I have been thinking about this and struggling with a decent solution. I think we shouldn't support what you suggest. We can provide "functions" to help configure an application (maybe even generalize the Config class so it can be reused) but what we are talking about is the object we pass to brigade so we shouldn't allow misuse. I'd suggest merging this as is and then discuss on a different PR a generic config class for users and a set of functions to let them configure their in a similar way as brigade can be configured.

In summary, I am merging this one and open an issue to track this new feature :)

@dbarrosop dbarrosop merged commit 81f42d2 into develop Dec 18, 2017
@dbarrosop dbarrosop mentioned this pull request Dec 18, 2017
@dbarrosop dbarrosop deleted the config_set_parameters branch December 21, 2017 12:49
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

2 participants