Hi,

thanks for keeping the ball rolling!

On 2014-08-06 02:51, Andy Parker wrote:
I'm pulling this discussion out into a new thread so that we can become
more focussed. I'm also going to start a thread about one other topic
that has been brought to my attention so that a decision can be reached.

In this thread I'd like to get to a decision about two aspects of
resource expressions:

   1. Whether to allow expressions in the type position ($a { hi: })

> The use case for number 1 is to provide determining the exact type of a
> resource at runtime. An example would be a module that had different
> implementations for debian vs redhat. It can then use parts of its self
> like so:
>
>    apache::install::$osfamily { 'something': }
>
> Note: I'm not promoting this or saying that this kind of construction
> should appear everywhere, but it is a feature that isn't available in
> the language at the moment.
>

What Trevor said: highly prone to misuse, but for the use case given, +1 as a shortcut to create_resources.

Note: we talked about $ a having various types, but I think this question only is about values that are Strings or Types, which I think is a very sensible restriction to avoid the complexities and abuses beyond the specified use case.

   2. Whether to allow using hashes as resources parameters
   3. If we allow hashes as resource parameters, what token introduces
it (no introducing token isn't an option because of implementation
constraints).

I really like Reid's suggestion to use a proper attribute name instead of an asterisk.

The use case for number 2 is determining dynamically what parameters
need to be included. In fact I saw a question just recently where
someone tried to do (something like):

   service { 'apache':
     if $control_service {
       ensure => running
     }
     enabled => true
   }

That could be done instead as:

   $params = if $control_service {
     { ensure => running }
    } else {
      { }
    }
    service { 'apache':
       * => $params;
     default:
       enabled => true
     }

This code gives me the hives. As I've replied in that thread, my usual way to write this is

  service { 'apache':
    enabled => true
  }

  if $control_service {
    Service['apache'] { ensure => running }
  }

If I were forced to use hashes, I think I'd write something along these lines:

  $service_params = {
    enabled => true
  }
  if $control_service {
    $service_params['ensure'] = 'running'
  }
  create_resource('service', $service_params)

Re-reading that, it could be even argued, that it flows better because the create_resource is at the end, and all relevant values are collected before, while my way adds values after the resource definition. Also the hash version doesn't duplicate the resource title. I'm beginning to like it even better.

To show Reid's proposal:

  $service_params = $control_service ? {
    true => { $ensure => 'running' }
    false => {}
  }

  service { 'apache':
    enable => true,
    defaults => $service_params;
  }

also very concise and readable to me.


A second use case is to provide a way of allowing local defaults to be
reused across multiple resources. And a third one was presented by Erik.

Are these use cases invalid or not needed? If not, how should they be
solved? There is create_resources. Is that really a good solution? I ask
that in all seriousness given that the only purpose of the puppet
language is to construct catalogs of resources to manage configurations
and the current syntax for those fundamental elements are so inflexible
that we had to add a function to leave the language for slightly more
advanced resource creation scenarios.

Puppet Labs has a UX department that is at the ready to perform any user
testing or usability analysis that anyone thinks might be valuable to
answer these questions. If we can work out what kinds of questions there
are we can definitely get some study done.

I do like both the defaults: title keyword and Reid's '(attribute|param)_(override|defaults|hash)' metaparam instead of the splat operator proposals. Both are shorthands for hash manipulation and a create_resource call. In the most general sense, the whole Puppet DSL is a shorthand for hash manipulation and create_resource calls. So for me the questions is, are those shorthands understandable and valuable, that is, more readable/modifyable/writeable than a hash+create_resource. Like Trevor has suggested, we all might not be the best suited to reason about this ;-)


Regards, David


On Tue, Aug 5, 2014 at 4:11 PM, Henrik Lindberg
<henrik.lindb...@cloudsmith.com <mailto:henrik.lindb...@cloudsmith.com>>
wrote:

    On 2014-05-08 18:24, Andy Parker wrote:

        On Tue, Aug 5, 2014 at 8:18 AM, Erik Dalén
        <erik.gustav.da...@gmail.com <mailto:erik.gustav.da...@gmail.com>
        <mailto:erik.gustav.dalen@__gmail.com
        <mailto:erik.gustav.da...@gmail.com>>> wrote:

             On 5 August 2014 16:25, Reid Vandewiele
        <r...@puppetlabs.com <mailto:r...@puppetlabs.com>
             <mailto:r...@puppetlabs.com <mailto:r...@puppetlabs.com>>>
        wrote:

                 On Mon, Aug 4, 2014 at 3:18 PM, Henrik Lindberg
                 <henrik.lindberg@cloudsmith.__com
        <mailto:henrik.lindb...@cloudsmith.com>
                 <mailto:henrik.lindberg@__cloudsmith.com
        <mailto:henrik.lindb...@cloudsmith.com>>> wrote:


                     So, to summarize: The use of * => as an operator is not
                     liked but the concept of being able to set
        attributes from a
                     hash is. Unfortunately, it is not possible to
        directly allow
                     an expression at the position in question, there
        must be a
                     syntactical marker.

                     As pointed out earlier, the * => was thought to read as
                     "any_attribute => from_these_values", but I totally
        grok if
                     people have an allergic reaction.

                     We can do this though:

                     file { default: ($hash) }

                     This works because it is impossible to have an
        attribute
                     name in parentheses.

                     In use:

                     file (
                        default   : ($my_file_defaults + { mode =>
        '0666' });
                        '/tmp/foo': ;
                        '/tmp/bar': ;
                     }

                     Is that better? No new operator, but you have to use
                     parentheses around the expression.

                     We can naturally also revert the functionality, but
        it seems
                     it is liked conceptually.


                     - henrik



                 I think the parenthesis are far preferable over *=>.
        That isn't
                 to say I like them - I don't particularly. But the
        functionality
                 is desirable, and if it's a matter of a technical
        limitation
                 then parenthesis are a Good Enough (TM) compromise from
        the more
                 ideal direct use of a hash.


             I really prefer the use of  * => over the parenthesis.
        Especially if
             you need to merge things into the hash. For example look at
        this
             example:

             # using parenthesis hash style
             class foo (

             $servername = $::fqdn,
             $port = 80,
             $ssl = false,
             $extra_opts={},

             ) {
             apache::vhost { $servername: ($extra_opts + {


             port => $port,
             ssl => $ssl,
             })

             }
             }
             # using * =>
             class foo (

             $servername = $::fqdn,
             $port = 80,
             $ssl = false,
             $extra_opts={},

             ) {
             apache::vhost { $servername:

             port => $port,
             ssl => $ssl,
             * => $extra_opts,


             }

             }


        The behavior that we worked out doesn't allow having the splat along
        with the other parameters, the reason being that it isn't clear
        what is
        meant by that. You had in your head that port and ssl are
        overridden by
        extra_opts (possibly because they come first?), but another,
        completely
        reasonable interpretation is that it is the other way around
        (port and
        ssl override extra_opts because they are explicitly given. In
        order to
        remove any of that confusion the current specification and
        implementation doesn't allow mixing. That can, I think, be changed.

        In the current implementation this would need to be:

        apache::vhost { $servername:
            * => $extra_opts + { port => $port, ssl => $ssl }
        }

             IMO the *=> is way more readable (as gist here if formatting is
             screwed up for you:
        https://gist.github.com/dalen/__57b37b80a9ba1879b78c
        <https://gist.github.com/dalen/57b37b80a9ba1879b78c>). This is quite
             similar to what I linked earlier that I am doing in
        
https://github.com/spotify/__puppet-puppetexplorer/blob/__master/manifests/init.pp#L89-__L97
        
<https://github.com/spotify/puppet-puppetexplorer/blob/master/manifests/init.pp#L89-L97>
             So it is not just a contrived example.


        My argument against using parenthesis is that parenthesis, are often
        read as "seldom necessary grouping". I believe that most programmers
        read them as usually only needed for fixing precedence problems,
        which
        is really what is happening here but it doesn't look like it.
        Based on
        that I can imagine that a common, and frustrating mistake would be:

            apache::vhost { $servername: $opts }

        And then confusion and anger and bug reports.


    Yeah, I think they are too subtle too (and hence the * =>).


    One more proposal :-)

    We could leave out the name part all together (i.e. drop the '*').

    dalens' example would then look like this:


          apache::vhost { $servername:

          port => $port,
          ssl  => $ssl,
               => $extra_opts,

    And if it is used for local defaults (or the only thing for a titled
    resource):

         file { default: => $hash }
         file { '/tmp/foo': => $hash }

    This works best if it is restricted to being the only attribute
    operation for a title, but looks a bit odd when presented in a list
    where there are also named (i.e. name => expression) operations.

    At least it is not a new operator.

    Is this better than * => or requiring parentheses ?


    - henrik

    --

    Visit my Blog "Puppet on the Edge"
    http://puppet-on-the-edge.__blogspot.se/
    <http://puppet-on-the-edge.blogspot.se/>

    --
    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 puppet-dev+unsubscribe@__googlegroups.com
    <mailto:puppet-dev%2bunsubscr...@googlegroups.com>.
    To view this discussion on the web visit
    
https://groups.google.com/d/__msgid/puppet-dev/lrrobu%245b1%__241%40ger.gmane.org
    
<https://groups.google.com/d/msgid/puppet-dev/lrrobu%245b1%241%40ger.gmane.org>.

    For more options, visit https://groups.google.com/d/__optout
    <https://groups.google.com/d/optout>.




--
Andrew Parker
a...@puppetlabs.com <mailto:a...@puppetlabs.com>
Freenode: zaphod42
Twitter: @aparker42
Software Developer

*Join us at PuppetConf 2014 <http://www.puppetconf.com/>, September
22-24 in San Francisco*
/Register by May 30th to take advantage of the Early Adopter discount
<http://links.puppetlabs.com/puppetconf-early-adopter> //—//save $349!/

--
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 puppet-dev+unsubscr...@googlegroups.com
<mailto:puppet-dev+unsubscr...@googlegroups.com>.
To view this discussion on the web visit
https://groups.google.com/d/msgid/puppet-dev/CANhgQXvw3AxSQgrUQQKpMyrCNPkdci9gytum-C832C7_q8v6xQ%40mail.gmail.com
<https://groups.google.com/d/msgid/puppet-dev/CANhgQXvw3AxSQgrUQQKpMyrCNPkdci9gytum-C832C7_q8v6xQ%40mail.gmail.com?utm_medium=email&utm_source=footer>.
For more options, visit https://groups.google.com/d/optout.


--
* Always looking for people I can help with awesome projects *
G+: https://plus.google.com/+DavidSchmitt
Blog: http://club.black.co.at/log/
LinkedIn: http://at.linkedin.com/in/davidschmitt

--
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 puppet-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/puppet-dev/53E1CA3D.4070509%40dasz.at.
For more options, visit https://groups.google.com/d/optout.

Reply via email to