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

bad ref to error fix #47487

Merged
merged 1 commit into from May 16, 2018

Conversation

Projects
None yet
5 participants
@rosscdh

rosscdh commented May 4, 2018

What does this PR do?

fixes a bad unboundvariable reference

What issues does this PR fix or reference?

#47486

Previous Behavior

throws error when error not defined

New Behavior

error is defined

Tests written?

No

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@terminalmage terminalmage merged commit 74a49c3 into saltstack:develop May 16, 2018

5 of 10 checks passed

jenkins/PR/salt-pr-linode-cent7-py3 Pull Requests » Salt PR - Linode CentOS 7 - PY3 #4683 — ABORTED
Details
jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests » Salt PR - Linode Ubuntu14.04 #22586 — ABORTED
Details
jenkins/PR/salt-pr-linode-ubuntu16-py3 Pull Requests » Salt PR - Linode Ubuntu16.04 - PY3 #9625 — ABORTED
Details
default Build finished.
Details
jenkins/PR/salt-pr-rs-cent7-n Pull Requests » Salt PR - RS CentOS 7 #18738 — FAILURE
Details
WIP ready for review
Details
codeclimate All good!
Details
jenkins/PR/salt-pr-clone Pull Requests » Salt PR - Clone #24858 — SUCCESS
Details
jenkins/PR/salt-pr-docs-n Pull Requests » Salt PR - Docs #16979 — SUCCESS
Details
jenkins/PR/salt-pr-lint-n Pull Requests » Salt PR - Code Lint #21599 — SUCCESS
Details
@terminalmage

This comment has been minimized.

Member

terminalmage commented May 16, 2018

Backported in #47672.

rallytime added a commit that referenced this pull request May 16, 2018

@davelindquist-egistix

This comment has been minimized.

davelindquist-egistix commented Jun 26, 2018

If I read this PR correctly, it doesn't quite work as desired?

The problem code:

try:
    __salt__['docker.inspect_image'](full_image)
    error = False
except CommandExecutionError as exc:
    msg = exc.__str__()
    ...

The solution simply moves the error=False prior to the try -- but shouldn't the solution look more like:

try:
    __salt__['docker.inspect_image'](full_image)
    error = False
except CommandExecutionError as exc:
    error = True
    msg = exc.__str__()
    ...

to capture the intent?

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