> > On Tue, Oct 19, 2010 at 7:59 PM, Markus Roberts <[email protected]>wrote: > >> >> >> >>> And finally, I'm not seeing why the dodge with the context thread var is >>>> needed. Isn't this just going to be the current AST node? >>>> >>>> >>> >>> Yes. But from lookupvar, we don't know what the current AST node is (as >>> far as I could find). The goal is simply to find the current line and file, >>> preferably from something that mixes in Util::Errors, so we can use >>> error_context. If there's somewhere more correct to get that information >>> from lookupvar, I would like to fix it, as it's a rather inelegant solution >>> now. >>> >> >> So two ideas: >> >> 1) The obvious idea would be to pass it in to lookupvar from the call site >> (which should know the context), or if you'd rather think outside the box >> 2) conditionally raise an exception when you detect the error and move the >> warning logic into safeevaluate, with a "retry" to resume after the message >> is generated. >> > > Regarding #1, lookupvar is called from a lot of places (ast evaluate > methods, extlookup, Puppet::Parser::Resource, and template processing), and > many of these don't have easy access to the current file and line number. > > #2 is dangerous because it involves retrying of code that potentially has > side effects. To be certain that it was safe, we would have to trace > through every code path from safeevaluate to lookupvar to make sure that it > was safe to execute twice. > > Although I'm not generally a fan of thread-local variables, I think this is > one situation where the advantages outweigh the disadvantages. I see two > major advantages of the method we chose: 1. since the thread-local variable > parallels safeevaluate's use of exception handling to tag error messages > with file and line numbers, we have good reason to be confident that file > and line number information will be applied to these warnings at least as > reliably as it is applied to errors (and that is known to work well). 2. > the method we chose will extend trivially to any warnings we may decide to > add in the future. >
-- 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.
