On Apr 19, 2012, at 1:49 PM, R.I.Pienaar wrote:

> 
> 
> ----- Original Message -----
>> From: "Andrew Parker" <[email protected]>
>> To: [email protected]
>> Sent: Thursday, April 19, 2012 9:43:36 PM
>> Subject: Re: [Puppet-dev] Changes to variable scoping in Telly
>> 
>> 
>> On Apr 19, 2012, at 1:28 PM, R.I.Pienaar wrote:
>> 
>>> 
>>> 
>>> ----- Original Message -----
>>>> From: "Andrew Parker" <[email protected]>
>>>> To: [email protected]
>>>> Sent: Thursday, April 19, 2012 8:42:33 PM
>>>> Subject: Re: [Puppet-dev] Changes to variable scoping in Telly
>>>> 
>>>> I've got a branch of puppet that has dynamic scoping removed.
>>>> Considering the impact that this will have on things and my
>>>> personal
>>>> uncertainty about whether people will still be able to achieve
>>>> everything they want to with puppet, I'd really like to get people
>>>> to grab it and try it out.
>>>> 
>>>> https://github.com/zaphod42/puppet/tree/feature/13970/remove-dynamic-scoping
>>> 
>>> This Just Works for me, but I've deliberately never had any dynamic
>>> lookup
>>> in my code as I've always used extlookup/hiera to avoid this
>>> problem so thats
>>> not a surprise, someone else will need to test with more broken
>>> manifests :P
>>> 
>> 
>> Yeah, I expect that this will bite people a lot more who didn't
>> religiously hold themselves to certain patterns.
>> 
>>> So I am guessing the thing that would happen is variables that were
>>> previously
>>> filled in would be undef without dynamic scope or what would be the
>>> the exact
>>> effects of this?
>>> 
>> 
>> Good question, it should be undef, but I'll add a test for that. In
>> fact I'll take all of the old tests for the old dynamic behavior and
>> update them for what should happen now.
> 
> there's been a recurring question about a strict mode for puppet - so that
> it will, when enabled, cause a compile failure when you try to read a undef
> variable ie. "foo-${bar}" would compile fail if bar is undefined.
> 
> This seems like something that might help a lot in catching problems if indeed
> the behaviour is as you say above
> 

A strict mode would help quite a bit.

Here are the two test cases that I added back in. They pass and show that you 
can get either nil or a different value than what you used to depending on the 
situation.

  describe "in situations that used to have dynamic lookup" do
    it "ignores the dynamic value of the var" do
      expect_the_message_to_be('node_msg') do <<-MANIFEST
          node default {
            $var = "node_msg"
            include foo
          }
          class baz {
            $var = "baz_msg"
            include bar
          }
          class foo inherits baz {
          }
          class bar {
            notify { 'something': message => $var, }
          }
        MANIFEST
      end 
    end 

    it "finds nil when the only set variable is in the dynamic scope" do
      expect_the_message_to_be(nil) do <<-MANIFEST
          node default {
            include baz
          }
          class foo {
          }
          class bar inherits foo {
            notify { 'something': message => $var, }
          }
          class baz {
            $var = "baz_msg"
            include bar
          }
        MANIFEST
      end 
    end 
  end 

It would be possible to give a warning when it would have found a different 
value via the dynamic lookup. We could keep that it Telly and remove the 
warning later to help people find cases where the new lookup rules are finding 
something different. Essentially just flip things around a little bit from what 
is in 2.7. Instead of performing both lookups warning when they are different 
and then returning the result of the dynamic, we could do both lookups, warn if 
different, and then return the result of the "twoscope" lookup.

> 
>> 
>>> Do you have sample code showing what worked before and failed
>>> after? and how
>>> would that failure/change in behaviour manifest?
>>> 
>> 
>> That is something that we'll need to put together so that there is
>> clear documentation about what changes are going to be needed to be
>> made in the new system.  As a first pass, the test cases should
>> cover those situations.
>> 
>>>> 
>>>> On Apr 19, 2012, at 11:45 AM, R.I.Pienaar wrote:
>>>> 
>>>>> 
>>>>> 
>>>>> ----- Original Message -----
>>>>>> From: "Andrew Parker" <[email protected]>
>>>>>> To: [email protected]
>>>>>> Sent: Thursday, April 19, 2012 7:16:40 PM
>>>>>> Subject: Re: [Puppet-dev] Changes to variable scoping in Telly
>>>>>> 
>>>>>> 
>>>>>> On Apr 19, 2012, at 9:38 AM, R.I.Pienaar wrote:
>>>>>> 
>>>>>>> ----- Original Message -----
>>>>>>>> From: "Andrew Parker" <[email protected]>
>>>>>>>> To: [email protected]
>>>>>>>> Sent: Thursday, April 19, 2012 5:30:29 PM
>>>>>>>> Subject: Re: [Puppet-dev] Changes to variable scoping in Telly
>>>>>>>> 
>>>>>>>> Absolutely. Debugging is always made fiendishly difficult when
>>>>>>>> there
>>>>>>>> is "action at a distance" stuff going on. Limiting that kind
>>>>>>>> of
>>>>>>>> interaction is why globals are frowned upon in most
>>>>>>>> programming.
>>>>>>>> Is
>>>>>>>> there a lot of use of globals (topscope) other than facts (and
>>>>>>>> enc
>>>>>>>> parameters, I guess) in puppet?
>>>>>>>> 
>>>>>>> 
>>>>>>> Today there is a lot of it going around.  The bulk of modules
>>>>>>> would
>>>>>>> be
>>>>>>> written with something like:
>>>>>>> 
>>>>>>> class foo::install {
>>>>>>> if $foo_version { $version = $foo_version } else { $version =
>>>>>>> "present"}
>>>>>>> 
>>>>>>> package{"foo": ensure => $version}
>>>>>>> }
>>>>>>> 
>>>>>>> and then people will "configure" these modules either in
>>>>>>> site.pp
>>>>>>> with
>>>>>>> globals or in a node with node scope variables.
>>>>>>> 
>>>>>>> node "blah" {
>>>>>>> $foo_version = "1.2.3"
>>>>>>> 
>>>>>>> include foo::install
>>>>>>> }
>>>>>>> 
>>>>>>> This is very common, it used to be the only real option outside
>>>>>>> of
>>>>>>> using
>>>>>>> extlookup and hiera.  And there are vast amounts of code out in
>>>>>>> the
>>>>>>> real
>>>>>>> world and on the forge built around this pattern.
>>>>>>> 
>>>>>>> worse, you also had:
>>>>>>> 
>>>>>>> class someother {
>>>>>>> $foo_version = "1.2.4"
>>>>>>> 
>>>>>>> include foo::install
>>>>>>> }
>>>>>>> 
>>>>>>> and so depending on the parse order of the actual include lines
>>>>>>> the
>>>>>>> outcome
>>>>>>> might be different in the end, this being at the whim of the
>>>>>>> autoloader and
>>>>>>> such it was a bit tricksy.
>>>>>>> 
>>>>>>> All of this code either needs big refactors or just doesnt work
>>>>>>> with 2.7.
>>>>>>> 
>>>>>> 
>>>>>> Thanks for these examples, they help on lot in my understanding
>>>>>> of
>>>>>> common patterns (I hope at some point to go through code on the
>>>>>> forge and try to get some more understanding).
>>>>>> 
>>>>>> Since I'm not entirely clear on all the changes from 2.7 to 2.6,
>>>>>> what
>>>>>> changed to make these things stop working in 2.7? From my
>>>>>> understanding of the 2.7 variable lookup system it seems like
>>>>>> they
>>>>>> should work in 2.7.
>>>>> 
>>>>> My examples isnt showing the particular bug I was thinking about
>>>>> -
>>>>> I
>>>>> will need to go dig through IRC logs to find the particulars.
>>>>> 
>>>>> However in the above examples accessing $foo_version would log a
>>>>> deprecation
>>>>> warning saying you must use fully qualified variable path.  But
>>>>> for
>>>>> node
>>>>> variables there is no fully qualified variable path so you're
>>>>> just
>>>>> stuck
>>>>> and forced to make peace with a all the warnings since there's no
>>>>> non
>>>>> param classes way to fix that.  This is less drastic than 'doesnt
>>>>> work' but
>>>>> for many its unacceptable either way as no clear way exist to fix
>>>>> this dire
>>>>> warning of impending future doom:
>>>>> 
>>>>> warning: Dynamic lookup of $nodevar at /home/rip/test.pp:4 is
>>>>> deprecated.
>>>>> Support will be removed in Puppet 2.8.  Use a fully-qualified
>>>>> variable name
>>>>> (e.g., $classname::variable) or parameterized classes.
>>>>> 
>>>>> The proposal that started this thread is about that but I believe
>>>>> its the
>>>>> wrong way, we need a node scope so people can address node scope
>>>>> variables
>>>>> different from top scope variables (what will happen if in a node
>>>>> I
>>>>> set the
>>>>> value of a fact if node scope vars become top scope vars?)
>>>>> 
>>>>> And we're suggesting rather than new and wonderful ways to
>>>>> address
>>>>> scopes
>>>>> ie $::topscopevar lets just use hashes.
>>>>> 
>>>>> In puppet code we'd have:
>>>>> 
>>>>> $facts["operatingsystem"]
>>>>> 
>>>>> in templates we'd have:
>>>>> 
>>>>> @facts["operatingsystem"]
>>>>> 
>>>>> all nice and simple vs the current situation of
>>>>> $::operatingsystem
>>>>> vs
>>>>> scope.lookupvar("::operatingsystem")
>>>>> 
>>>>>> 
>>>>>>> Until there is wide adoption of param classes or some data
>>>>>>> system
>>>>>>> this
>>>>>>> will unfortunately still be the way.
>>>>>>> 
>>>>>>> The proposed hash like syntax will make for much lighter
>>>>>>> refactoring apart
>>>>>>> from all the other gains expressed in that ticket.
>>>>>>> 
>>>>>> 
>>>>>> Is the main thing standing in the way of param class adoption
>>>>>> the
>>>>>> changes that you referenced above that are keeping people moving
>>>>>> to
>>>>>> 2.7? Or are they in some way inadequate (Jo mentions this in his
>>>>>> email)?
>>>>> 
>>>>> I think a capable data system is required to make param classes
>>>>> viable, whats
>>>>> wrong with them would be an entirely different thread (of which I
>>>>> think there
>>>>> are several in the list archives probably approaching 100s of
>>>>> messages :P)
>>>>> 
>>>>> --
>>>>> You received this message because you are subscribed to the
>>>>> Google
>>>>> Groups "Puppet Developers" group.
>>>>> To post to this group, send email to [email protected].
>>>>> To unsubscribe from this group, send email to
>>>>> [email protected].
>>>>> For more options, visit this group at
>>>>> http://groups.google.com/group/puppet-dev?hl=en.
>>>>> 
>>>> 
>>>> --
>>>> You received this message because you are subscribed to the Google
>>>> Groups "Puppet Developers" group.
>>>> To post to this group, send email to [email protected].
>>>> To unsubscribe from this group, send email to
>>>> [email protected].
>>>> For more options, visit this group at
>>>> http://groups.google.com/group/puppet-dev?hl=en.
>>>> 
>>>> 
>>> 
>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups "Puppet Developers" group.
>>> To post to this group, send email to [email protected].
>>> To unsubscribe from this group, send email to
>>> [email protected].
>>> For more options, visit this group at
>>> http://groups.google.com/group/puppet-dev?hl=en.
>>> 
>> 
>> --
>> You received this message because you are subscribed to the Google
>> Groups "Puppet Developers" group.
>> To post to this group, send email to [email protected].
>> To unsubscribe from this group, send email to
>> [email protected].
>> For more options, visit this group at
>> http://groups.google.com/group/puppet-dev?hl=en.
>> 
>> 
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "Puppet Developers" group.
> To post to this group, send email to [email protected].
> To unsubscribe from this group, send email to 
> [email protected].
> For more options, visit this group at 
> http://groups.google.com/group/puppet-dev?hl=en.
> 

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to