Skip to content

Fix 54501#56381

Merged
dwoz merged 9 commits intosaltstack:masterfrom
mchugh19:54501
Apr 18, 2020
Merged

Fix 54501#56381
dwoz merged 9 commits intosaltstack:masterfrom
mchugh19:54501

Conversation

@mchugh19
Copy link
Copy Markdown
Contributor

What does this PR do?

When state.py runs the _run_check function to validate unless/onlyif, the resulting output can include a list as a comment:

{'result': True, 'comment': ['unless condition is true'], 'skip_watch': True}

This list was breaking the state output when test=True with retry, which assumed any existing comments would already be strings. This PR add six.text_type casting to to string formatting to support other input types.

What issues does this PR fix or reference?

#54501

Previous Behavior

Running a state with included unless and retry with test=True would result in crash:

 Traceback (most recent call last):
         File "/root/gits/salt/tests/unit/test_state.py", line 265, in test_verify_retry_parsing
           self.assertDictContainsSubset(expected_result, state_obj.call(low_data))
         File "/root/gits/salt/salt/utils/decorators/state.py", line 30, in _func
           result = func(*args, **kwargs)
         File "/root/gits/salt/salt/state.py", line 2084, in call
           low['retry']['splay'])])
       TypeError: sequence item 0: expected str instance, list found

New Behavior

Output is rendered as expected:

['unless condition is true']  The state would be retried every 5 'seconds (with a splay of up to 0 seconds) a maximum of 5 times or until a result of True is returned

Tests written?

Yes

Commits signed with GPG?

No

@mchugh19 mchugh19 requested a review from a team as a code owner March 15, 2020 08:27
@ghost ghost requested a review from Akm0d March 15, 2020 08:27
@mchugh19
Copy link
Copy Markdown
Contributor Author

Possibly fixes #52800 as well. An example state was not supplied for that issue, but the line referenced in the exception is the same issue as #54501.

DmitryKuzmenko
DmitryKuzmenko previously approved these changes Apr 2, 2020
@DmitryKuzmenko DmitryKuzmenko self-assigned this Apr 2, 2020
@DmitryKuzmenko
Copy link
Copy Markdown
Contributor

@mchugh19 could you please resolve merge conflicts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ZRelease-Sodium retired label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants