Inconsistency in master_call interface for Runner/Wheel clients

[hunternet93]

• salt.runner.RunnerClient().master_call() is async and only returns a JID.
• salt.wheel.WheelClient().master_call() is sync and returns the full return.

We should support both event-driven and non-event-driven interfaces, of course!

[whiteinge]

Thanks for filing this. This broke in e1b4ef0.

Edit: No it didn't. It was already broken there too (wheel != runner).

[whiteinge]

@hunternet93 So APIClient() is not yet ready for a public release. We'll get there but in the meantime the *Client() interfaces are the recommended Python entry point -- don't worry, they're not missing any functionality.

I fleshed out the *Client() docs a bit and included examples:

http://docs.saltstack.com/ref/clients/index.html

That said, currently salt.runner.RunnerClient().master_call() is hard-wired for async behavior. It should be able to do both sync and async. This took a little refactoring but I have a pull req. nearly ready to go. I'll update this ticket when it goes in but I wanted to drop a quick status update.

[whiteinge]

I edited the ticket description now that the problem has been clarified. The pull req is ready pending discussion. I will send it tomorrow.

https://github.com/whiteinge/salt/compare/refactor-master_call

[whiteinge]

The previous pull req was off-base since that part of Salt is very sensitive to blocking code (which runners are); everything there should be async. There is already a precedent for a synchronous interface wrapping an async call by watching the event bus in LocalClient().cmd_cli(). The proper fix for this ticket is to implement the same in RunnerClient() and WheelClient().

Since we're on the topic, there are other inconsistencies between the *Client() interfaces. It's not clear which methods support eauth and which don't. It's not always clear which methods are async and which are sync. Here's a short summary:

LocalClient()

• cmd() - eauth / sync (by way of blindly waiting for the timeout duration then returning whatever)
• cmd_cli() - eauth / sync (by way of intelligently listening to the event bus until the job is done)
• run_job() - eauth / async (returns job ID)

RunnerClient()

• cmd() - sync (blocking)
• low() - sync (blocking)
• async() - async (returns job ID)
• master_call() - eauth / async (wraps async())

WheelClient()

• call_func() - sync (blocking)
• master_call() - eauth / sync (blocking)

[whiteinge]

This is blocking a high-severity bug. Changing priority accordingly.

[cachedout]

Looks like a flurry of activity here but it's unclear if this is ready to be closed. @whiteinge?

[whiteinge]

Ah, thanks. This can indeed be closed.

Note, there's a problem with the master_call interface for runners. But that is, as they say, another issue. #15218

[cachedout]

Thanks for the follow-up @whiteinge. Much appreciated!