theforeman / foreman_ansible_inventory

Foreman dynamic inventory script for ansible - Now merged into Ansible itself
GNU General Public License v3.0
70 stars 31 forks source link

Complex data types in parameters #17

Open arielsalvo opened 8 years ago

arielsalvo commented 8 years ago

Hi,

I needed to support JSON and YAML as data types as parameters and I worked it out as simple parameters with a prefix.

something like: param: "#@@JSON@@# { 'test': 'hello world' }"

or: param: "#@@YAML@@#---\n'test': 'hello world'"

Are you interested in the code? I can also pick up the prefixes from the ini file.

agx commented 8 years ago

On Wed, Jun 22, 2016 at 10:44:57AM -0700, Ariel Salvo wrote:

Hi,

I needed to support JSON and YAML as data types as parameters and I worked it out as simple parameters with a prefix.

something like: param: "#@@JSON@@# { 'test': 'hello world' }"

or: param: "#@@YAML@@#---\n'test': 'hello world'"

Are you interested in the code? I can also pick up the prefixes from the ini file.

While I like the idea the syntax scares me a bit, to be honest.

Wouldn't it be cleaner to allow for more complex parameter types by associating a mime type with each parameter (defaulting to text/plain? This would require some changes on the foreman side (or a plugin) to allow for more complex parameter types but this would then be available to everybody, not only the ansible inventory.

I'm currently mostly using the parameters to tag some identity on the hosts like:

application: foobar
domain: whatever
tier: web

and get the rest from (version controlled) group vars. Being able to retrieve more complex data from the foreman sounds good though for some use cases.

Cheers, -- Guido

arielsalvo commented 8 years ago

Mime types sound great but it seems from the redmine backlog that complex type support has been requested repeatedly for a long time now and my dev-skills are not enough to help much in that regard.

Do you think it is bound to happen in the short run?

If we implement this now, it would be part of the "fallback to string" and it can be deprecated a few versions if/when mime types are implemented in Foreman and then removed. Plus, when that happens, it should be quite easy to create a "sanity check" script to walk over all parameters using the API and warn if this then-deprecated format should be upgraded in any of them.

Any thoughts?

dLobatog commented 8 years ago

Not sure how you envision these mime types being served by the API? I'm generally OK with the proposal. Although instead of providing these prefixes, one can try to parse first JSON, then YAML, then fallback to string if that doesn't work, right?

arielsalvo commented 8 years ago

The option to try to parse without the prefixes did come up but I chose against it at the time. The rationale was that, AFAIK, it would be impossible to pass a string containing json or a yaml because it would always be parsed. Does that make sense?

arielsalvo commented 8 years ago

I missed your question, dLobatog....

I don't think the API should serve the mime types themselves. I see it as adding a way to select the mimetype in foreman and storing it its database. Then, the API would use this field in the same way I'm proposing for my prefix.

agx commented 8 years ago

On Fri, Jul 01, 2016 at 04:31:18AM -0700, Daniel Lobato García wrote:

Not sure how you envision these mime types being served by the API? I'm generally OK with the proposal. Although instead of providing these prefixes, one can try to parse first JSON, then YAML, then fallback to string if that doesn't work, right?

Guessing might break existing installations. What about a

'all_parameters_types':

that has the mime types of non string parameters? -- Guido