Is MultiEval.evals insane?

[goodboy]

The following is posted verbatim from @dtkerrs review of #49 with regard to the switchy.distribute.MultiEval interface and implementation:

So some feedback here on evals is that it might be nicer to not use string-based commands in the various calls. If your evals call used the __code__ attribute, if it exists, on the expression you could rewrite something like

def is_alive(self):
    return any(self.pool.evals('listener.is_alive()'))

as

def is_alive(self):
    def alive():
        listener.is_alive()
    return any(self.pool.evals(alive))

which I think is nicer personally.

I haven't looked at all of it, but if the only goal of evals is to let you call methods or get attributes from the listener or client it might also make more sense to rewrite evals to let you call it something like either of these:

def is_alive(self):
    return any(self.pool.evals(lambda listener: listener.is_alive()))

def is_alive(self):
    def slave_alive(listener):
        return listener.is_alive()
    return any(self.pool.evals(slave_alive))

Which can work since you can extract the name of arguments from a code object by looking at co_varnames

---

@vodik also commented:

I think I'd agree with @dtkerr suggestion because I can't see how eval could be faster than a function call. Evaluating code should have more overhead, no?

Looking at your code, I see how and why you do the evals - to run the same snippet with different globals/locals, but I don't see how any of that is desirable over function calls...

[idletea]

So re: this that you said elsewhere:

def is_alive(self):
    def alive():
        listener.is_alive()
    return any(self.pool.evals(alive))

has to lookup listener from somewhere. eval makes this simple by just handing it a namespace but invoking functions means probably building functools.partials or something similar.

That example works just fine if you only change one thing: instead of assuming the evals argument is a string, check if it has __code__ defined on it, and if so send that to eval instead. The namespace injection thing in eval works just fine with both string and code objects being evalled.

The idea I think would be better goes a bit further: instead of injecting into the namespace using eval you can require that the call to pool.evals use a function/lambda and extract the named arguments it expects from callable.__code__.co_varnames. That means you can just write the inline lambda like

def is_alive(self):
    return any(self.pool.evals(lambda listener: listener.is_alive()))

then in the evals call you'd have something like

if func.__code__.co_varnames[0] == "listener":
    func(listener)
elif func.__code__.co_varnames[0] == "client":
    func(client)

and then you don't even need the eval call. (And it'd probably make more sense if that's what you do to rename evals to run_on_slaves or something.

[goodboy]

I'm seriously on board with this after having used salt more extensively now.
Hopefully this can get in for a next minor release :)

@dtkerr I'd of course take a PR if you feel so inclined ;)