Use data types instead of validate functions #163
Conversation
manifests/lvs/real_server.pp
Outdated
| String[1] $virtual_server, | ||
| Stdlib::IP::Address $ip_address, | ||
| Stdlib::Port $port, | ||
| Hash $options = {}, |
There was a problem hiding this comment.
Looking at https://github.com/voxpupuli/puppet-keepalived/blob/master/templates/_format_options.erb it looks like we can have a defined type type Keepalived::Options = Hash[String, Variant[Hash, String, Boolean]]. Technically the latter Hash is a Keepalived::Options but I don't know if Puppet supports recursive types.
There was a problem hiding this comment.
I think it'd need to be
Keepalived::Options = Hash[String, Variant[Hash, String, Boolean, Integer]]
Not sure it's worth it.
There was a problem hiding this comment.
Sure, ok. I'd imagine 90% of the times where we're currently using Hash we could use this instead. It's probably quite rare to use anything other than a string as the hash key.
The primary aim of this PR was just to get rid of the validate functions. There are a few classes that still have no types at all.
alexjfisher
changed the title
| Stdlib::Filemode $config_dir_mode = $keepalived::params::config_dir_mode, | ||
| Stdlib::Filemode $config_file_mode = $keepalived::params::config_file_mode, | ||
|
|
||
| String[1] $config_group = $keepalived::params::config_group, |
There was a problem hiding this comment.
Why not Variant[String[1], Integer] ? This will permit to use GID also .
There was a problem hiding this comment.
It might be very rare anyone wants to do this and GID as a string does already work. Dunno if it just complicates the type with no real benefit.
There was a problem hiding this comment.
I am thinking about this because with root i always use 0.
But i am fine also with KIS.
alexjfisher
force-pushed
the
remove_validate
branch
3 times, most recently
from
9d6539d
to
79a878e
Compare
alexjfisher
changed the title
|
I'll merge #162 first (after it goes green), then rebase this. |
alexjfisher
force-pushed
the
remove_validate
branch
from
79a878e
to
5ce7c9e
Compare
This is a breaking change as several parameters now only accept integers instead of strings. Fixes voxpupuli#159
This is a breaking change as several parameters now only accept integers
instead of strings.
Fixes #159