Hi list,
As an exercise to get me familiar with the new type system I started a
from-scratch rewrite of the apt module (don't worry I won't publish it like
that). The reasons for this are legion but it's mostly an exercise to get
acquainted with the type system and rethink a few of the things we're
doing, try out new stuff etc. All my testing has been done with Puppet
3.7.1's version of the future parser.
First, some code (if the arrows don't line up it's a font thing, they line
up in Vim just fine):
class apt::params {
$purge_defaults = { 'sources_file' => true,
'sources_dir' => true,
'preferences_file' => true,
'preferences_dir' => true, }
$update_defaults = { 'policy' => 'changed',
'timeout' => undef,
'tries' => undef, }
$proxy_defaults = { 'host' => undef,
'port' => 8080, }
}
class apt (
Hash[Enum[sources_file,
sources_dir,
preferences_file,
preferences_dir], Boolean] $purge = {},
Struct[{policy => Optional[Enum[changed, always, daily, weekly]],
timeout => Optional[Integer],
retries => Optional[Integer],}] $update = {},
Struct[{host => Optional[String],
port => Optional[Integer[1,65535],}] $proxy = ${},
Hash $sources = {},
Hash $keys = {},
) inherits apt::params {
$merged_purge = merge($purge_defaults, $purge)
$merged_update = merge($update_defaults, $update)
$merged_proxy = merge($proxy_defaults, $proxy)
}
As a rationale for all the hashes... What I want to have is not four
purge_* parameters but one purge parameter with four keys. The user should
then be allowed to supply a partial hash (meaning only the keys that need
to change from the default) to configure behaviour (this is what the three
merge() calls achieve in the body).
First of all, this is awesome. Being able to express what kind of input you
expect this way is great. Especially because you can go to great lengths to
make sure that what you receive is what you would expect. Even though the
Type code might be a bit much at this point it's going to spare us a whole
load of headaches later on because we can just use a variable/value and be
sure that it's set to something sensible. Hurray!
Unfortunately, if you look at the type code, even for the first Hash, it
quickly becomes difficult to read. It would be great if we could alias
these things somehow, along the lines of:
type purge_validation = Hash[Enum[sources_file, sources_dir,
preferences_file, preferences_dir], Boolean]
class apt {
Purge_validation $purge = $::apt::params::purge_defaults,
) inherits apt::params { }
I don't have a preference for a keyword but type seems sensible, alias
could probably work too. Even though the validation code is still quite
complex the class declaration itself becomes mighty easy to read, about as
easy as the current form without all the type annotations.
By the way, if you're wondering what that Type declaration says: I expect a
Hash, that can contain 0-* keys (I did not specify a size on the hash so it
is allowed to be empty). Those keys must be named one of these four things
(those in the Enum[]) and I expect all values associated with those keys to
be Boolean, so true or false.
This means people can no longer torture you and your beautiful module with
crap like this:
class { 'apt':
purge => { 'source_file' => 'yes', 'sources_dir' => 'false',
'preference_file' => 'UNDEF', 'preferences_dir' => true },
}
Puppet will simply throw errors at them. The errors themselves aren't very
informative though. Currently you get them in the form of: Expected
parameter 'purge' to have type <the whole type definition here> but got
<something else>. It would be nice if we could get error messages along the
lines of: Expected parameter 'purge' to be <a more human description> but
got <another more human description> I know that's a tall order, but on the
list of "nice to have" I suppose.
Onwards! Optional is causing me some trouble. According to the blog
"Luckily, the type system has a type called Optional that does exactly what
we want in this situation, it accepts something of a specific type or
Undef."
This would mean that notice(undef =~ Optional[Numeric]) should evaluate to
true, and indeed it does:
Notice: Scope(Class[main]): true
Notice: Compiled catalog for nvc2542 in environment production in 0.33
seconds
Notice: Finished catalog run in 0.01 seconds
So this should also work:
class test (
Optional[Numeric] $number = undef,
) {}
include test
However, it does not:
Error: Expected parameter 'number' of 'Class[Test]' to have type
Optional[Numeric], got Runtime[ruby, Symbol] on node nvc2542.
I tried to change that to Variant[Numeric, Undef], even though that's
exactly what Optional is defined as, but no dice either. This feels like a
bug to me, I'm hoping Henrik or Andy can shed some light on the situation.
One really awesome sauce feature of Optional is when it comes to hashes.
Earlier I showed this piece of code:
Struct[{policy => Optional[Enum[changed, always, daily, weekly]],
timeout => Optional[Integer],
retries => Optional[Integer],}] $update = {},
What I'm defining here is that I want a Hash (Struct[{}]), whose keys are
named 'policy', 'timeout' and 'retries'. By setting their values to
Optional however you are now allowed to pass in a partial hash, so just
sending { 'policy' => 'changed' } into update will work without it
complaining that you're missing the 'timeout' and 'retries' keys.
One thing that does strike me as slightly odd though is that even though
the value is defined as Optional, which should allow us to send in undef,
you cannot. If you do { 'policy' => undef } you'll get an error based on
that validation. Now, I'm very glad it doesn't allow me to do so because
that's really the behaviour I want in this specific case, but that might
not always be true.
There are places where I really would like to allow undef as a value for a
hash key, but not always. I haven't found a way to express that yet though.
So essentially want to be able to say both:
- key may be omitted but if available must be of value Integer <- current
behaviour of Optional[Integer] with a Structs{{ key => }]
- key may be omitted but if available may be Integer or Undef <- I can't
seem to express this. Though there is no need in the case of $update to
pass in '{retries => undef}' a user should be allowed to do so even if it
doesn't achieve anything and from my understanding of the Optional
definition, it should.
Lets add a bit to my confusion:
class test (
Struct[{policy => Optional[Enum[changed, always, daily, weekly]],
timeout => Optional[Integer],
retries => Optional[Integer],}] $update = {}
) {
}
include test
23:35:37 ~/D/g/d/p/apt (master) $ puppet apply test.pp --parser future
Notice: Compiled catalog for nvc2542.nedap.local in environment production
in 0.41 seconds
Notice: Finished catalog run in 0.01 seconds
It gets a bit weirder because this is valid too:
class test (
Struct[{policy => Optional[Enum[changed, always, daily, weekly]],
timeout => Optional[Integer],
retries => Optional[Integer],}] $update = { 'retries' => undef, }
) {
}
include test
23:35:58 ~/D/g/d/p/apt (master) $ puppet apply test.pp --parser future
Notice: Compiled catalog for nvc2542.nedap.local in environment production
in 0.42 seconds
Notice: Finished catalog run in 0.01 seconds
But:
class test (
Struct[{policy => Optional[Enum[changed, always, daily, weekly]],
timeout => Optional[Integer],
retries => Optional[Integer],}] $update = {}
) {
}
class { 'test':
update => { 'retries' => undef, 'policy' => 'changed', 'timeout' => 1},
}
Results in:
23:38:01 ~/D/g/d/p/apt (master) $ puppet apply test.pp --parser future
Error: Expected parameter 'update' of 'Class[Test]' to have type
Struct[{'policy'=>Optional[Enum['changed', 'always', 'daily', 'weekly']],
'timeout'=>Optional[Integer], 'retries'=>Optional[Integer]}], got
Hash[String, Runtime[ruby, Symbol]] at
/Users/daenney/Development/github/daenney/puppet/apt/test.pp:9 on node
nvc2542.nedap.local
At this point I'm utterly confused. It looks like I can't have Optional
accept undef on a 'top' parameter, I can use it on a hash and initialise
that hash with a key that is set to undef but I cannot pass in a hash with
that same key set to undef.
Maybe I've misunderstood the behaviour of Optional or something is going
wrong in the way undef is being parsed, the Runtime[ruby, Symbol] I find
very suspicious, but someone should look at this and figure out what's
going on. I'm betting the answer is going to be "you're being an idiot" but
I would really like to understand why.
Except for my troubles with Optional all I have to say is "sweeeeeeeeet".
As a module maintainer, this will prevent a lot of headaches. If only
puppet-strings could parse a human-understandable description of the Type
annotation into the docs it generates... :).
--
Daniele Sluijters
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/puppet-dev/c51a97d0-e773-40e7-9831-7a881a2f99b9%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
