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

Types are inconsistently converted #5

Open
knorby opened this issue Sep 16, 2013 · 2 comments
Open

Types are inconsistently converted #5

knorby opened this issue Sep 16, 2013 · 2 comments
Labels
Milestone

Comments

@knorby
Copy link
Owner

knorby commented Sep 16, 2013

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
Copy link

+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
Copy link
Owner Author

knorby commented Sep 21, 2014

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.

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

No branches or pull requests

2 participants