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

6922 fix network error errors #51343

Merged
merged 2 commits into from
Jan 27, 2019

Conversation

waynew
Copy link
Contributor

@waynew waynew commented Jan 25, 2019

What does this PR do?

Changes the error message producing code to actually produce the expected behavior (i.e. provide a useful error message).

It would be nice to refactor the debian_ip and rh_ip states, they have a lot of shared code, but I think it's probably fine for now to just make this fix. If I have to touch these modules again, though, I'll probably take the time to refactor out all the shared code 😛

What issues does this PR fix or reference?

#6922

Previous Behavior

Bad options produced a stack trace, whoops.

New Behavior

Bad options provide a nice helpful comment, e.g.:

  Comment: Invalid network setting -- Setting: networking, Expected: [yes|on|true|1|True|no|off|false|0|False]

Tests written?

Yes

Commits signed with GPG?

Yes

Right now it fails, badly, when an invalid value was passed, obscuring
the real problem(s).

This makes it str-ify the values first.

Signed-off-by: Wayne Werner <wwerner@saltstack.com>
Previously, like the debian module, this would blow up with invalid
values. Now it shows the proper error message. It looks like the
incorrect behavior was introduced in the commit on

Wed Apr 4 18:31:00 2012 -0600 - 45aaf46

Signed-off-by: Wayne Werner <wwerner@saltstack.com>
@dwoz dwoz merged commit 3d4c195 into saltstack:develop Jan 27, 2019
garethgreenaway added a commit to garethgreenaway/salt that referenced this pull request Sep 19, 2019
@waynew waynew deleted the 6922-fix-network-error-errors branch November 6, 2019 20:23
dwoz added a commit that referenced this pull request Nov 12, 2019
@crlane
Copy link

crlane commented Jan 8, 2020

Will this fix be in the next release? It does not appear to be in 2019.2.2. Any idea when that might be?

@waynew
Copy link
Contributor Author

waynew commented Jan 24, 2020

@crlane this should show up in the Neon release - we've already got an RC out that you can try!

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.

3 participants