[PATCH 0/2] Fix #1088 - collection overrides
Brice Figureau
Here is an updated version of my previous fix for #1088.
It takes into account what Luke and I discussed previously on this
_except_ the language change, so that basically leaves us:
* collection can override some parameters of collected resources.
* collection applies on virtual and non-virtual resources
The current version is available here:
http://github.com/masterzen/puppet/tree/tickets/0.24.x/1088
Please review and comment,
Brice Figureau (2):
Fix #1088 - Collections should have a facility for modifying found
resources
Fix #1088 - part2 - Add rspec tests
lib/puppet/parser/ast/collection.rb | 31 +
lib/puppet/parser/collector.rb | 68 ++-
lib/puppet/parser/grammar.ra | 22 +-
lib/puppet/parser/parser.rb | 1455 +++++++++++++++--------------
spec/unit/parser/ast/collection.rb | 63 ++
spec/unit/parser/collector.rb | 159 +++-
test/data/snippets/collection_override.pp | 8 +
test/language/snippets.rb | 6 +
8 files changed, 1054 insertions(+), 758 deletions(-)
create mode 100755 spec/unit/parser/ast/collection.rb
create mode 100644 test/data/snippets/collection_override.pp
Luke Kanies
> Hi,
>
> Here is an updated version of my previous fix for #1088.
> It takes into account what Luke and I discussed previously on this
> _except_ the language change, so that basically leaves us:
>
> * collection can override some parameters of collected resources.
> * collection applies on virtual and non-virtual resources
Crap. I just realized that this will actually be essentially
impossible to use. Take a very simple case:
class one {
notify { "I'm in one": }
}
class two {
notify { "I'm in two": }
Notify <| |> { loglevel => warning }
}
include one, two
(Imagine files instead of 'notify'.)
You'll always get this error:
Only subclasses can override parameters
Because the collection in 'two' operates on all resources, but can't
actually override them.
For a more realistic example:
class other {
notify { "I'm in other": }
}
class base {
notify { "I'm in base": }
}
class sub inherits base {
notify { "I'm in sub": }
Notify <| |> { loglevel => warning }
}
include other, sub
Again, people will want to use this downward, like defaults work, but
it only actually works upward. Even worse, it can work downward, but
doing so will only ever result in an error.
Not only will this not get people what they want, it will essentially
be useless except for resource types that are only used within a
single class heirarchy, and even then only for limited cases.
I'm sorry, I should have seen this earlier.
I don't think we can apply this until we can find a way to make it
work downward instead of upward.
--
Normal is getting dressed in clothes that you buy for work and driving
through traffic in a car that you are still paying for - in order to
get to the job you need to pay for the clothes and the car, and the
house you leave vacant all day so you can afford to live in it.
-- Ellen DeGeneres
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com
Brice Figureau
Yes, I thought something was wrong since the beginning but couldn't put
a finger on the exact issue.
> I don't think we can apply this until we can find a way to make it
> work downward instead of upward.
Without looking at the code, I'd say that something along the line of
what follows could work:
* do not rely on resource override system (of course since that's the
issue)
* recognize that resource collection is a _global_ operation, no matter
if it is deep in the class hierarchy, you'll always collect resources
defined elsewhere or in another branch of the class hierarchy (and this
even when you collect only virtual resources).
* then we could decide to apply the override differently: as a
"collection override" instead of resource override. Those overrides
could take place in the compiler finish method and thus take precedence
on any other override or resource definition or default.
Does it make sense?
--
Brice Figureau
Days of Wonder http://www.daysofwonder.com/
Luke Kanies
>
>> I don't think we can apply this until we can find a way to make it
>> work downward instead of upward.
>
> Without looking at the code, I'd say that something along the line of
> what follows could work:
>
> * do not rely on resource override system (of course since that's the
> issue)
>
> * recognize that resource collection is a _global_ operation, no
> matter
> if it is deep in the class hierarchy, you'll always collect resources
> defined elsewhere or in another branch of the class hierarchy (and
> this
> even when you collect only virtual resources).
>
> * then we could decide to apply the override differently: as a
> "collection override" instead of resource override. Those overrides
> could take place in the compiler finish method and thus take
> precedence
> on any other override or resource definition or default.
I'm afraid that this explodes the complexity. What if a user has
conflicting collection overrides? Which wins? Does that one always
win?
The whole reason behind Puppet's inheritance system is built around
answering that question, because it's not an easy one to answer.
--
Brand's Asymmetry:
The past can only be known, not changed. The future can only be
changed, not known.
Brice Figureau
> On Dec 1, 2008, at 6:57 AM, Brice Figureau wrote:
> >
> >> I don't think we can apply this until we can find a way to make it
> >> work downward instead of upward.
> >
> > Without looking at the code, I'd say that something along the line of
> > what follows could work:
> >
> > * do not rely on resource override system (of course since that's the
> > issue)
> >
> > * recognize that resource collection is a _global_ operation, no
> > matter
> > if it is deep in the class hierarchy, you'll always collect resources
> > defined elsewhere or in another branch of the class hierarchy (and
> > this
> > even when you collect only virtual resources).
> >
> > * then we could decide to apply the override differently: as a
> > "collection override" instead of resource override. Those overrides
> > could take place in the compiler finish method and thus take
> > precedence
> > on any other override or resource definition or default.
>
> I'm afraid that this explodes the complexity. What if a user has
> conflicting collection overrides? Which wins? Does that one always
> win?
None, it should fail with an exception, because it is a conflict and
since we can't guarantee parse order it shouldn't be allowed.
> The whole reason behind Puppet's inheritance system is built around
> answering that question, because it's not an easy one to answer.
You are right, but unfortunately collections as they are right now don't
fit this model. This is not an issue per-se, since they basically are
"read-only" injection of resources, but it is still a global operation.
Anyway, I'll think about this a little bit more to see if I can come
with a better plan :-)
If you have any ideas, let me know. The same for the other languages
discussions you had with some other people on this list (about
parametrization of classes or resource ordering constraints).
Meanwhile I guess it is time to work on 0.25, isn't it?
I still have some rough cutting-edge not working rest status handler to
finish (ok, it works but does nothing and it couldn't even pass the
rspec puppetmasterd test) :-)
Luke Kanies
I guess this would be a reasonable compromise -- you get one shot, and
if you need to change the value again, it's an exception.
It brings up some other questions -- simple ones, but ones that must
be answered. Most importantly, it seems, is whether these would
actually override existing values or only add new values; in other
words, whether they behave more like defaults or overrides. I think
they would be overrides, but... I can just see someone wanting to use
a collection/override, and then wanting to change the override:
class debian {
$collection = Package <| |> { provider => aptitude }
}
class ubuntu inherits debian {
$collection { provider => synaptic }
}
Something like that. Obviously that syntax isn't anywhere near
working atm, but you get the idea. And it's almost reasonable to
think about changing the collection, rather than changing the
resources multiple times. That is, you have a single Package
collection, which you then modify, and in the end it gets applied. Hmm.
>
>> The whole reason behind Puppet's inheritance system is built around
>> answering that question, because it's not an easy one to answer.
>
> You are right, but unfortunately collections as they are right now
> don't
> fit this model. This is not an issue per-se, since they basically are
> "read-only" injection of resources, but it is still a global
> operation.
>
> Anyway, I'll think about this a little bit more to see if I can come
> with a better plan :-)
Great.
>
> If you have any ideas, let me know. The same for the other languages
> discussions you had with some other people on this list (about
> parametrization of classes or resource ordering constraints).
>
> Meanwhile I guess it is time to work on 0.25, isn't it?
> I still have some rough cutting-edge not working rest status handler
> to
> finish (ok, it works but does nothing and it couldn't even pass the
> rspec puppetmasterd test) :-)
Heh. Yeah, 0.25 is now the priority. I'll be working on getting all
the tests in master to pass again, at which point I'll largely start
pushing for a release candidate. I'm going to scan the tickets
assigned to 0.25 and see which ones I can fit in, but I know I'll be
bumping a lot of them because I think it's more important to get this
release out than just about anything else right now -- we need to
converge these development lines or I'm going to explode.
--
As a general rule, don't solve puzzles that open portals to Hell.