bugfix: convert integer strings to integer

[vicinus]

with puppet 4 the following is no longer true: 1 == '1'. So with puppet 4 it
must be ensured, that when variables are compared either both are integer or
both are strings.

[3flex]

I think it would be better to change the defaults from strings to integers, and introduce a better validation, like:

  if (!is_integer($listen_port)) or (is_string($listen_port)) {
    fail('$listen_port must be an integer.')
  }

I do thank you for adding tests!

@jfryman what's your preference?

[vicinus]

I would also prefer to enforce real integers and have no problem changing the code. But I also thought about this and decided against it for backward compatibility. I expect that there are a lot of users who have configured there ports with string values (because the defaults are string values) and they all will have to change them. But I could be wrong or you prefer a painful break than to draw out the agony. 😏

Probably changing the default values to integer and adding a deprecation warning for string values would be the right move at the moment...

[jfryman]

@3flex I really would like to move to actual types to stop trying to mangle strings. Lots of new language advancements we can/should take care of.

[3flex]

@vicinus I like the deprecation idea. So do your hack for now so Puppet 4 works, add a deprecation warning, then we can clean up in next release + 1.

  if is_string($listen_port) {
    warning('DEPRECATION: String $listen_port must be converted to an integer. Integer string support will be removed in a future release.')
  }
  elsif !is_integer($listen_port) {
    fail('$listen_port must be an integer.')
  }

[vicinus]

I have added the deprecation warnings. Please let me know if there is anything else I can do.

[3flex]

Looks good! If you could also update /manifests/resource/streamhost.pp and /manifests/resource/upstream/member.pp then we'll merge.

Thanks!

[vicinus]

I updated /manifests/resource/streamhost.pp and /manifests/resource/upstream/member.pp.

[3flex]

Excellent! Thanks for this. If you spot other issues like this in the module PRs are definitely welcome to nudge things closer to best practices.

[jhoblitt]

Is there any chance of getting a 0.3.x release that includes this PR? The vhost fragment construction is completely broken under at least puppet 4.4.1 without this.