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

RFC: Refactor of the playbook package #52

Closed
wants to merge 5 commits into from

Conversation

andrioni
Copy link
Contributor

Hi!

I opened this PR in order to propose a small refactor of the playbook package. The idea is make getting the text of the playbook and parsing it more orthogonal, in order to support both new playbook providers (e.g. REST, ZooKeeper, etcd) and different formats (e.g. JSON, TOML).

If you are OK with the changes, I plan to add support for getting the playbooks from a REST interface. The GET request would accept both JSON and YAML, and automatically parse the playbook depending on the Content-Type of the response.

jbeemster and others added 5 commits November 13, 2015 16:22
This refactor focuses on making adding new playbook formats or
providers easier in the future.
@andrioni
Copy link
Contributor Author

Also, another possible change in the future (probably not for series of patches) would be to try to load/parse all the queries right after the playbook is loaded: this would help to isolate any provider-related stuff, and could be a bit more stable (e.g. the connection to Consul might not be available during a later step, or the query itself might have changed).

@alexanderdean
Copy link
Member

Hey @andrioni - on the second point, we have a ticket for that: #22

@alexanderdean
Copy link
Member

This looks cool to me @andrioni ! I like the approach of refactoring to support multiple discrete back-ends... @jbeemster thoughts when you are back?

@andrioni
Copy link
Contributor Author

andrioni commented Dec 3, 2015

Nice! With this and #22, I'll get back to working on a REST interface. Right now I'm thinking about an API pretty identical to the consul one, re-using the -playbook and the -sqlroot arguments.

@alexanderdean
Copy link
Member

Sounds good @andrioni ! Thanks for your contributions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants