search

knorby / facterpy

Python library to provide a cached and dictionary-like interface to Puppet's facter utility
BSD 2-Clause "Simplified" License
8 stars 4 forks source link

Types are inconsistently converted #5

Open knorby opened 11 years ago

knorby commented 11 years ago

Facts already have inconsistent types and are usually strings more often then not, regardless of the value. When using with YAML, some values show up as a non-string type, and when not using YAML, they show up as a string. There are a few ways to improve this:

  1. Only return strings and expect fact-aware libraries to handle them appropriately. This option removes the type information that facter can embed and sacrifices the improved interface with YAML for the limitations of the fallback parser.
  2. Follow a model like ConfigParser's and add type-specific get methods. Considering that gathering all systems facts is a handled use case, this option doesn't handle all potential issues.
  3. Attempt to coerce types based on a simple heuristic. Integers and booleans would likely always be correct, but floats are more challenging, as, particularly in the context of facter, "^([0-9]+.{1}[0-9]+)$" can capture things like version numbers and other values that are meant as strings.

The best option is probably a choice between 1 and 3.

Olivia5k commented 10 years ago

+1 for something that would do casting. It feels weird to cast my stuff in my work.

Maybe we can utilize json.loads() somehow? That would fix most default cases and is fairly straightforward.

(thanks for a cool and useful little module, btw)

knorby commented 10 years ago

Well, the type issue mostly exists because yaml support can't be guaranteed, but you can get it if you initialize Facter with use_yaml. I don't remember the issues around facter support (I don't use puppet regularly anymore), but yaml support in python was the other reason. I recommend that you use the yaml approach if this is a concern for you, as this approach actually gets the correct type from facter.

I think there are some cases where json.loads wouldn't work out, which gives me some pause on that particular approach; something more robust than a json parser could certainly work here. As I mentioned in the issue, I'm also concerned with cases like version strings.

I can fit something in, but I will probably leave it off by default, similar to how the yaml flag works.