-
Notifications
You must be signed in to change notification settings - Fork 40
Convert tar_extract to a class #488
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really see the benefit of this.
This also allows the
beforerelationship to properly build the correct dependency chain.
There already was a before present so what's different?
From what I can tell - the before isn't respected properly when it's a define. These tests don't show it, but I encountered this when I re-factored the code pulling apart some relationships. If we don't want to merge this now, I'll open up my bigger re-factor and can include the change there instead. |
IMHO that implies a bug in Puppet.
I'd be tempted to go that route. |
|
Ok, now I have run into this issue which I knew was coming once I entangled all my changes. The PR is failing with the below output. With this PR applied on top of #488 the tests pass. |
|
metaparameters like You can validate which dependencies are actually set by using the graphviz .dot files that Puppet can generate.
|
|
How will that explain or help me understand why changing from a define to class solves the dependency problem? If |
|
@ehelms it will help understand what relationships are actually being created. Here's a minimal test case for what I think you're trying to validate, a defined type that sets relationships on something from within the type. define thing ($string) {
notify { $string:
before => Notify['last'],
require => Notify['first'],
}
}
notify {'last':}
thing {'a defined type':
string => 'foobar',
}
notify {'first': }It generates this relationship graph, demonstrating that the expected relationships are correctly being set. If you want more gory details, with all the intermediate graph nodes and such, you can use the What I suspect you're running into is the classic containment problem. The summary of that is that if you put a In other words this snippet of Puppet does not put any ordering relationship on Class['bar'] at all. class foo {
class { 'bar': }
}
notify { 'words':
before => Class['foo'],
}I have a fairly long blog post that explains how and why it works that way, if you're interested. |
|
Thanks for the lessons @binford2k - I must admit I still think I only understand 25% of what is going on. I have implemented an update using |
|
with the caveat that I'm not 100% sure what you're solving for, that PR seems reasonable. fwiw, I'm happy to help review PRs or even do a "weird parts of Puppet" training or some such. We just added training to our services offered (haven't updated the page yet) |



This only happens a single time, and is key to the execution when supplied. This also allows the
beforerelationship to properly build the correct dependency chain.