Hi,

+1
I really like this approach of this problem. I think Felix's original patch is much better than what I did years ago. It fixes the problem without the regression I had introduced. Patch seems really clean and as all Puppet tests are passing without problem I'm really confident in this patch.

Aurélien

Le 16/07/2015 09:06, Romain F. a écrit :
Hello,

Necrobumping again this thread. Felix's wishes have been granted in PR-4038 <https://github.com/puppetlabs/puppet/pull/4038> (PUP-4760 <https://tickets.puppetlabs.com/browse/PUP-4760>) but this change is bit tricky/risky apparently.

The original goal was to not retrieve ensure property status when we don't ask to. This need a change in Puppet::Type's retrieve method. Before the change in PR-4038 <https://github.com/puppetlabs/puppet/pull/4038>, it was programmatically creating a ensure property when nothing is specified and if the type is ensurable, so it was always retrieving the ensure status.

The change in the beginning of this thread was adding another condition to this : if the type is ensurable and if the ensure property have a should attribute.

The change in PR-4038 <https://github.com/puppetlabs/puppet/pull/4038> is just skipping the creation of the ensure property when nothing is specified and if the type can continue without a ensure property (it's the case for Services, not for Files for example).

Like Felix's patch, it doesn't break any tests and it doesn't break puppet resource <...> (which uses collected resources)

Can you confirm that this would work ? Do think it can be extended to some other types ?

Cheers,

Romain




Le lundi 5 mai 2014 02:16:33 UTC+2, Felix Frank a écrit :

    Hi,

    necro-bumping yet another thread, I took another stab at that old
    problem.

    I molded Jeff's approach into a form that I hope to be more
    palatable.
    It does not break any tests that I have tried (which is not saying it
    won't break any whatsoever).

    https://github.com/ffrank/puppet/tree/dont-always-retrieve-service-ensure


    So, if you guys could give it a spin, that would be awesome.

    Also, a ticket would be helpful, but if you can confirm that this
    works
    and helps, I can open one on your behalf so we can make a PR for
    this.

    Cheers,
    Felix

    On 12/19/2013 11:39 PM, Jeff Bachtel wrote:
    >>>
    >>> In the end, even just the behavior change to "puppet resource"
    makes
    >>> the patch a non-starter because it is a widely used feature.
    >>
    >> I understand this feature should be kept, but that a pity this
    should
    >> impact other even more useful feature like "apply" or "agent".
    >>
    >> Could it be possible that "puppet resource" and other like
    "apply" or
    >> "agent" retrieves only what they need? In apply/agent case,
    this come
    >> from a transaction being applied. For "puppet resource" I
    assume this
    >> is not the case. Could method parameter solve this case? And this
    >> could even keep the compat if this param is not specified
    >>
    >
    > I spent all day (because my Ruby is bad) doing a proof of
    concept with
    > this. It touches type.rb and indirector/resource/ral.rb to add a
    > seemingly transparent method variable flagging whether ensure
    should
    > be ignored for the purpose of retrieving resources. It defaults to
    > false (don't ignore ensure), which should cause the speedup
    Aurelien
    > is needing. The resource RAL indirector is aware of the method
    > parameter and sets it to true (ignoreensure), thereby exhibiting
    the
    > old behavior when puppet resource is called from the command line.
    >
    > There is a nasty bit that I'm unsure of in the retrieve_resource
    > method. I discovered that when running puppet agent -t --noop, if I
    > tried to use my newly defined method parameter that parsing would
    > choke - apparently in that state the retrieve method is targeting
    > another method. I worked around it by putting in a rescue statement
    > and falling back to the old way of calling retrieve which should,
    > eventually, still hit the retrieve with Aurelien's improved
    > conditional logic.
    >
    > Anyway, please find attached a diff containing the changes in
    > question. Feel free to refine and submit it as a PR, my Ruby really
    > isn't up for my doing so myself.
    >
    > Jeff
    >
    >

--
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/d846f39c-753f-4aaa-99f0-947822791c50%40googlegroups.com <https://groups.google.com/d/msgid/puppet-dev/d846f39c-753f-4aaa-99f0-947822791c50%40googlegroups.com?utm_medium=email&utm_source=footer>.
For more options, visit https://groups.google.com/d/optout.

--
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/55AF52C9.7080908%40cea.fr.
For more options, visit https://groups.google.com/d/optout.

Reply via email to