> So essentially all the cases where we used to use 'undef' to specify a
form of 'nothingness' so that the default for that parameter on the type we
were passing it is was respected are now handled by undef.
Eh, I meant to say "are now handled by default".
On Friday, 10 October 2014 00:06:31 UTC-7, Daniele Sluijters wrote:
>
> 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/92edd3e3-cf1b-4644-b865-8f0b7e5221f2%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
