The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

Feature #4561

Structured data should be supported

Added by Luke Kanies over 3 years ago. Updated 4 months ago.

Status:AcceptedStart date:05/18/2012
Priority:NormalDue date:05/18/2012
Assignee:-% Done:

0%

Category:library
Target version:2.0.0
Keywords:backlog Affected Facter version:
Branch:https://github.com/kbarber/facter/tree/ticket/master/4561-add_structured_data

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com

This ticket is now tracked at: https://tickets.puppetlabs.com/browse/FACT-151


Description

Facter currently only supports a flat result list, and it should instead support structured data – basically, a hash of hashes and arrays.

This should probably be able to be represented in two ways – either a hash of hashes/arrays, or namespaces. That is, this:

{:top => {:middle => {:bottom => "value"}}}

Could also be represented as:

top::middle::bottom = value

We could optionally have a converter that s/::/_/g for backward compatibility, which should be both trivial and unnecessary, but there should definitely be some kind of backward compatibility mode for callers who can’t deal with structured data.


Related issues

Related to Facter - Feature #5193: Facter should output in JSON Closed 11/03/2010
Related to Facter - Feature #4468: Virtual fact should return a list for hosts that can supp... Accepted 08/04/2010
Related to Facter - Bug #13291: Can't reconstitute subinterface/VLAN names from facter. Accepted 03/21/2012
Related to Facter - Bug #7547: Facter allows the creation of facts that Puppet/Ruby coul... Accepted 05/16/2011
Related to PuppetDB - Feature #20403: Support structured facts Accepted
Related to Facter - Feature #20408: Should be possible to merge structured facts Accepted
Duplicated by Facter - Bug #3704: Facter doesn't return booleans (converts them to strings ... Duplicate 04/29/2010
Blocks Facter - Feature #2847: mountpoint fact Code Insufficient 11/21/2009
Follows Facter - Feature #14570: Separate fact definition from fact resolution Accepted 05/17/2012

History

#1 Updated by James Turnbull about 3 years ago

  • Target version set to 14

#2 Updated by Adrien Thebo over 2 years ago

  • Assignee set to Adrien Thebo

#3 Updated by Adrien Thebo over 2 years ago

  • Status changed from Accepted to In Topic Branch Pending Review
  • Branch set to https://github.com/adrienthebo/facter/tree/ticket/master/4561-add_structured_data

#4 Updated by R.I. Pienaar over 2 years ago

With JSON becoming more and more prevalent is it worth considering specifically not supporting symbols as they cause problems with JSON?

>> {"foo" => "bar", :foo => "baz", "foo" => "meh"}.to_json
=> "{"foo":"baz","foo":"meh"}"

#5 Updated by Adrien Thebo over 2 years ago

  • Status changed from In Topic Branch Pending Review to Code Insufficient

Ah, I had not considered that. This feature also isn’t terribly helpful until puppet can receive richer data types, and there are probably other concerns like the caveat you mentioned. It seems reasonable to pull symbols out though.

#6 Updated by Ken Barber over 2 years ago

In regards to using the command line tool for displaying facter output – how do people feel about the format?

I’ve managed to rig up something very simply using pretty_inspect … thereby keeping the existing ruby-like output but allowing us to see hierarchy:

test1 => ["asdf", "fdsa"]
test2 => {"asdf"=>"fdsa"}
test3 => {"asdf"=>["asdf", "asdf"]}
test4 => [{"asdf"=>"fdsa"}, "bleah"]
test5 => {"k"=>"b",
  "l"=>"c",
  "a"=>"b",
  "m"=>"d",
  "b"=>"c",
  "n"=>"f",
  "c"=>"d",
  "e"=>"f",
  "g"=>"b",
  "h"=>"c",
  "i"=>"d",
  "j"=>"f"}

We could always switch over to YAML being the default – but I kind of like the output of pretty print. JSON is out of the question since its not in Ruby core.

The caveat with this mechanism is that strings are now surrounded in quotes:

uptime => "4 days"
uptime_days => 4

Since Adrian’s code catches anything that isn’t an object it should look okay.

#7 Updated by James Turnbull over 2 years ago

I think i’m okay with that – albeit I’d like to see what more and multi-layered facts look like.

#8 Updated by Ken Barber over 2 years ago

In that case let me do more less-contrived samples and paste them here for people to see.

#9 Updated by Ken Barber over 2 years ago

Here is a quick sample for network interfaces (static data – not auto generated):

network_interfaces => {"eth0"=>
  {"type"=>"physical",
   "ip_address"=>"192.168.1.1",
   "netmask"=>"255.255.255.0",
   "mac_address"=>"00:10:24:f4:ab:ce"},
 "eth1"=>
  {"type"=>"physical",
   "ip_address"=>"192.168.1.2",
   "netmask"=>"255.255.255.0",
   "mac_address"=>"00:10:24:f4:ab:cb"},
 "bond0"=>
  {"type"=>"bonding",
   "ip_address"=>"192.168.1.3",
   "netmask"=>"255.255.255.0",
   "mac_address"=>"00:10:24:f4:ab:cd",
   "interfaces"=>["eth0", "eth1"]}}

#10 Updated by Luke Kanies over 2 years ago

IMO the format has to be either json or yaml – some cross-language standard format. Nothing else is remotely portable.

If we don’t want to introduce a dependency, we could fake it with a simple implementation of the json subset we use.

#11 Updated by Ken Barber over 2 years ago

Well this is just for the visual output only. Are you more concerned about if we ever port Facter to another language Luke? Obviously if another language wants to fork facter and deserialize the output the —yaml/—json options can be used.

It wouldn’t be impossible to write a presentable output parser – since its only one way. We could write a json-like parser to avoid dependencies like you say. Or we could just change the default output to YAML and totally drop the existing output format. The latter is obviously the easy path – and would require almost no maintenance going forward.

#12 Updated by Ken Barber over 2 years ago

For visual assessment here is a YAML snippet using network_interfaces sample above plus some other data:

network_interfaces:
  eth0:
    type: physical
    ip_address: 192.168.1.1
    netmask: 255.255.255.0
    mac_address: 00:10:24:f4:ab:ce
  eth1:
    type: physical
    ip_address: 192.168.1.1
    netmask: 255.255.255.0
    mac_address: 00:10:24:f4:ab:cb
  bond0:
    type: bonding
    ip_address: 192.168.1.1
    netmask: 255.255.255.0
    mac_address: 00:10:24:f4:ab:cd
    interfaces:
      - eth0
      - eth1
timezone: BST
uptime: 4 days
uptime_days: 4
uptime_hours: 115

#13 Updated by Luke Kanies over 2 years ago

Ken Barber wrote:

Well this is just for the visual output only. Are you more concerned about if we ever port Facter to another language Luke? Obviously if another language wants to fork facter and deserialize the output the —yaml/—json options can be used.

Other tools already want to be able to consume Facter’s output, and a big part of what people like about Ohai is that portable output, so it has nothing to do with rewriting Facter and everything to do with making it easy to integrate other tools with Facter.

I consider this capability — easy parsing by arbitrary tools in arbitrary languages — to be required for every data type we produce.

It wouldn’t be impossible to write a presentable output parser – since its only one way. We could write a json-like parser to avoid dependencies like you say. Or we could just change the default output to YAML and totally drop the existing output format. The latter is obviously the easy path – and would require almost no maintenance going forward.

#14 Updated by Ken Barber over 2 years ago

Luke Kanies wrote:

I consider this capability — easy parsing by arbitrary tools in arbitrary languages — to be required for every data type we produce.

So what do you think about making the default output just YAML from here on in?

The existing format is pretty much only parse-able by Ruby – JSON means we need the JSON gem, and because I’ve had push back on providing default/core functionality with optional gems I think having YAML as the default makes sense here. (This doesn’t mean that JSON can’t be achieved as the —json switch that @lusis added does this already obviously).

Does anyone have a problem with this approach? Should I perhaps approach the community (maybe puppet-users) and ask them? This is really a UX question as its for human consumption as well – maybe Randall should add his 2 cents.

#15 Updated by Randall Hansen over 2 years ago

A good principle is that output should be human-readable by default. YAML is quite good for this, as long as we substitute “geek” for “human.” :) That’s appropriate for our audience.

#16 Updated by Adrien Thebo over 2 years ago

+1 for using YAML. It’s the simplest and most portable solution from what I can see.

#17 Updated by Luke Kanies over 2 years ago

YAML works great for all the needs I know of.

I think JSON will be required at some point, but yaml is the best of the available options at this point, IMO.

#18 Updated by Ken Barber over 2 years ago

So due to recent changes due to whitespace and branching I’ve just rebased off master and pushed into Adrien’s topic branch.

#19 Updated by Adrien Thebo over 2 years ago

  • Priority changed from High to Low

#20 Updated by Ken Barber about 2 years ago

  • Status changed from Code Insufficient to Accepted
  • Assignee changed from Adrien Thebo to Ken Barber
  • Branch changed from https://github.com/adrienthebo/facter/tree/ticket/master/4561-add_structured_data to https://github.com/kbarber/facter/tree/ticket/master/4561-add_structured_data

#21 Updated by Ken Barber about 2 years ago

  • Priority changed from Low to High

#22 Updated by Ken Barber about 2 years ago

  • Target version changed from 14 to 186

#23 Updated by Daniel Pittman about 2 years ago

  • Target version deleted (186)

#24 Updated by Walter Heck about 2 years ago

Wanted to add this to #3704, but that has been closed as a duplicate of this. On the geppetto mailing list the following problem popped up, which I think is particularly painful:

It seems all references to "false" and "true" trigger a warning in geppetto.  Since "true" != true and "false" != false this should probably be removed ( but I think is why it was added also ;)

The primary issue is that all facts that are true/false are "true"/"false" in recipes.  I also have some goofy exec resources where 'command => "true"' which flags a warning.

Example:

Some test code:

  if $::is_virtual == false {
    notify{ 'a': }
  } else {
    notify{ 'b': }
  }
  if $::is_virtual == "false" {
    notify{ 'c': }
  } else {
    notify{ 'd': }
  }

On a system:
# facter -p | grep is_virtual
is_virtual => false
# puppet agent --test | grep -i notify
notice: /Stage[main]/Common/Notify[b]/message: defined 'message' as 'b'
notice: /Stage[main]/Common/Notify[c]/message: defined 'message' as 'c'

#25 Updated by Ken Barber almost 2 years ago

  • Assignee deleted (Ken Barber)

#26 Updated by Jeff Weiss almost 2 years ago

  • Target version set to 2.1.0

#27 Updated by Jeff Weiss almost 2 years ago

  • Target version changed from 2.1.0 to 3.0.0

#28 Updated by eric sorenson over 1 year ago

  • Target version changed from 3.0.0 to 2.0.0

Pulling this back in for facter 2.

#29 Updated by eric sorenson over 1 year ago

  • Keywords set to backlog

#30 Updated by eric sorenson over 1 year ago

  • Priority changed from High to Normal

#31 Updated by eric sorenson about 1 year ago

I’ve written up what I think are the latest discussion as a puppet Armature (enhancement proposal process); i’m particularly curious to see how watchers on this bug feel about the two proposals in the “Alternatives” section:

https://github.com/puppetlabs/armatures/blob/master/arm-5.structured_facts/structured_facts.md

Please feel free to comment on these either by forking and submitting pull requests (if you want to add/change text) or raising github issues on the repository if it’s just commentary/questions.

#32 Updated by Erik Dalén about 1 year ago

eric sorenson wrote:

I’ve written up what I think are the latest discussion as a puppet Armature (enhancement proposal process); i’m particularly curious to see how watchers on this bug feel about the two proposals in the “Alternatives” section:

https://github.com/puppetlabs/armatures/blob/master/arm-5.structured_facts/structured_facts.md

Please feel free to comment on these either by forking and submitting pull requests (if you want to add/change text) or raising github issues on the repository if it’s just commentary/questions.

It seems that proposal mostly discusses changing the namespace for facts, I don’t really see why that is necessary for supporting this?

That belongs more to #11915

#33 Updated by Glen Ogilvie 5 months ago

This feature would be really helpful. It will allow people to use facts for things like system monitoring as well much better, when structured data, like a list of file systems, etc, can be returned.

Any idea on when this might end up available for use?

#34 Updated by Jason Antman 4 months ago

Redmine Issue #4561 has been migrated to JIRA:

https://tickets.puppetlabs.com/browse/FACT-151

Also available in: Atom PDF