Salt: LocalClient should not use print()

[root360-AndreasUlm]

Description of Issue/Question

I want to use salt.client.LocalClient() class in a script which iterates over a bunch of registered environemnts and runs commands.
As we have many Saltmaster it happens that a registered environment does not return data as no minion is connected the current Saltmaster.

Unfortunately LocalClient does not raise an exception which I could catch when no minion is connected for an environment but prints a message directly:
https://github.com/saltstack/salt/blob/1c9ebe2005daf9d79a8d70eca9de882d9ffb9128/salt/client/__init__.py#L282

There are other cases where print is used too:

• https://github.com/saltstack/salt/blob/1c9ebe2005daf9d79a8d70eca9de882d9ffb9128/salt/client/__init__.py#L213
• https://github.com/saltstack/salt/blob/1c9ebe2005daf9d79a8d70eca9de882d9ffb9128/salt/client/__init__.py#L273
• https://github.com/saltstack/salt/blob/1c9ebe2005daf9d79a8d70eca9de882d9ffb9128/salt/client/__init__.py#L1585
• https://github.com/saltstack/salt/blob/1c9ebe2005daf9d79a8d70eca9de882d9ffb9128/salt/client/__init__.py#L1586
• https://github.com/saltstack/salt/blob/1c9ebe2005daf9d79a8d70eca9de882d9ffb9128/salt/client/__init__.py#L1588

IMHO all those should be changed to either use the 'log'-object or to raise an exception based on the situation.
Examples:

• L213: should use 'log.error()' as an error happened but the whole process should not be stopped
• L273: should raise an exception as the call itself did not produce a response but the user should be able to react to this event, which currently is not possible as it's only a message on the stdout

Is there a reason why LocalClient uses print() instead of 'log'-object and exceptions?

Setup

Install salt-master and run with default configuration is sufficient.

Steps to Reproduce Issue

• run this script:

from salt.client import LocalClient
try:
  LocalClient().cmd('G@Minions:Empty', 'test.ping', tgt_type='compound', expr_form='compound')
except:
  print('error should be exceptable')

Versions Report

All version including 'develop' branch

[gtmanfred]

I have to say I agree with this, an error log would be better imho.

@saltstack/team-core can I get some opinions here?

Thanks!
Daniel

[DmitryKuzmenko]

Sounds reasonably. I like that idea.

[murmelantes]

I'm having the exact same issue, so I'm just leaving a quick comment so the issue doesn't auto close...
Thanks a lot!

[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.

[root360-AndreasUlm]

Bump to prevent closure of issue as the issue still exists e.g. in https://github.com/saltstack/salt/blob/master/salt/client/__init__.py#L281 and https://github.com/saltstack/salt/blob/develop/salt/client/__init__.py#L280

[stale[bot]]

Thank you for updating this issue. It is no longer marked as stale.

[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.

[root360-AndreasUlm]

@gtmanfred As this issue still persists and was accepted by the salt-core team can the stale-bot be deactivated for this issue?
Otherwise I have to add bumping comments every month just to prevent the automatic closure.

[stale[bot]]

Thank you for updating this issue. It is no longer marked as stale.

[cachedout]

@sagetherage Could you please add this to your triage list so that the correct lables are applied? Thanks.

[sagetherage]

@DmitryKuzmenko please put this on your list of tickets to triage this week - looks like it may have been lost in the blocked milestone and never had follow up.

[sagetherage]

@DmitryKuzmenko can you please triage this ticket, thanks!

[DmitryKuzmenko]

@sagetherage done.