Salt: Transition __salt__ to __runners__ in runners

[cachedout]

[rallytime]

When this transition happens, we should make sure the Dunder Dictionaries docs are updated - Specifically the "note" in the __salt__ section. (And add a new doc section for the new __runners__ dunder.)

[thatch45]

on the __runner__ concept, we would want to first make __runner__ available, move all of the core runners to it, and then make the deprecation error fire on any salt refs, then we could make it smooth

[carlpett]

Could you elaborate on the suggested change? Would there be a __runner__ dunder available to code running on the master (found this issue since that is exactly what I needed...)?

[cachedout]

@carlpett Right now, all runners which execute on the master have the __salt__ dunder available to them and this dunder is populated with the other runners so that functions from one runner can be cross-called by another. So if you need that functionality rght now, it's already there for you.

The idea here is simply to do some housekeeping by renaming that dunder to __runner_ and then bring in execution modules as __salt__.

[carlpett]

@cachedout Ah, I see. Sounds great!
My use case is slightly different - I want to call a runner from an execution module running on the master (the module is invoked as part of pillar rendering). I found saltutil.runner since writing my question, however, so all is well :) If the __runner__ dunder would be available for this case as well, that would make my code a bit more readable (it took almost an hour of "The following keyword arguments are not valid" until I found the right way to pass parameters, partly because it is _slightly_ different from passing arguments to publish.runner...)

[cachedout]

Ah, I see. For your case, saltutil.runner is the way to go. Good point as well about the disparity between the two functions. I _hate_ cases where we accept **kwargs into execution module functions so we should clean that up. Thanks for the reminder. :]

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[arthurlogilab]

If this is stale or closed, maybe the comment in https://github.com/saltstack/salt/blob/develop/salt/runners/fileserver.py#L86-L92 should be removed ?

[thatch45]

@arthurlogilab This has been reopened as an RFC and we are going to move forward with it from a fresh outlook.

[max-arnold]

@thatch45 Also it would be good to standardize on how the dunder is named:

• Doc: __runners__ (plural) https://docs.saltstack.com/en/latest/topics/development/modules/developing.html#runners
• Engines: __runners__ (plural) https://github.com/saltstack/salt/blob/c4d39dfc9ff2407e8cdddb6cef97fc5ba64b5099/salt/loader.py#L346
• Roster: __runner__ (singular) https://github.com/saltstack/salt/blob/c4d39dfc9ff2407e8cdddb6cef97fc5ba64b5099/salt/loader.py#L525
• Thorium: __runner__ (singular) https://github.com/saltstack/salt/blob/c4d39dfc9ff2407e8cdddb6cef97fc5ba64b5099/salt/loader.py#L535

Existing engines do not use this plural dunder, so I can make a PR against 2018.3 (or any other preferred branch) to rename it to __runner__.