Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not try to run git config when git is not available #169

Merged
merged 1 commit into from
Feb 24, 2018

Conversation

rhertzog
Copy link
Contributor

The state will not fail gracefully, instead you will get
an error like this one:

      ID: users_rhertzog_user_gitconfig_0
Function: git.config_set
    Name: alias.br
  Result: False
 Comment: State 'git.config_set' was not found in SLS 'users'
          Reason: 'git' __virtual__ returned False
 Changes:

And since pillar data can't be (easily) tuned according to minion's
status, we really need this check here.

My tests with Salt 2017.7.3 have shown that cmd.has_exec() is reliable
for this, contrary the what the comment was implying.

The state will not fail gracefully, instead you will get
an error like this one:

          ID: users_rhertzog_user_gitconfig_0
    Function: git.config_set
        Name: alias.br
      Result: False
     Comment: State 'git.config_set' was not found in SLS 'users'
              Reason: 'git' __virtual__ returned False
     Changes:

And since pillar data can't be (easily) tuned according to minion's
status, we really need this check here.

My tests with Salt 2017.7.3 have shown that cmd.has_exec() is reliable
for this, contrary the what the comment was implying.
@aboe76
Copy link
Member

aboe76 commented Feb 24, 2018

@rhertzog could you specify the salt version for this? to be sure it works

something like:

{% if grains['saltversioninfo'] >= [2017, 7, 3,0] %}
{% endif %}

@rhertzog
Copy link
Contributor Author

@aboe76 I don't think that such a test makes sense. If the command was not working in some old version, then it might have been fixed with a point release of that old version. And we would need correct information of what version broke because otherwise adding a supplementary test is just causing more troubles than anything else.

Besides I don't see how you would want to hook that test, it would mean to duplicate the content:

if good-version
  if has_exec
   state
  endif
else
  state
endif

... which I find really ugly.

@aboe76
Copy link
Member

aboe76 commented Feb 24, 2018

@rhertzog I think you are correct,

@aboe76 aboe76 merged commit e05d551 into saltstack-formulas:master Feb 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants