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

Fix issue 54755 and add regression tests #54826

Merged
merged 3 commits into from Oct 1, 2019

Conversation

@dwoz
Copy link
Contributor

dwoz commented Sep 30, 2019

What does this PR do?

Address #54755 and add regression tests.

In general we should try to address this kind of thing without mucking around with already imported modules. Ideally we'd recognize that pip was upgraded and re-start the minion automatically. This will however address the bugs reported against the pip_state and other states using the unless requisite.

What issues does this PR fix or reference?

#54755

Previous Behavior

If no pip module is in the python environment imports of the pip state failed with multiple tracebacks. This is due to us trying to del pip when there is no pip variable defined.

New Behavior

Use a purge_pip method before trying to import pip this method will only try to remove the pip module and/or pip variable if it exists.

Tests written?

Yes

Commits signed with GPG?

Yes

@Akm0d
Akm0d approved these changes Sep 30, 2019
@dwoz dwoz force-pushed the dwoz:issue_54755 branch from aaaadfc to a2b53d9 Oct 1, 2019
@dwoz

This comment has been minimized.

Copy link
Contributor Author

dwoz commented Oct 1, 2019

re-run full all

1 similar comment
@dwoz

This comment has been minimized.

Copy link
Contributor Author

dwoz commented Oct 1, 2019

re-run full all

dwoz added 2 commits Sep 30, 2019
@dwoz dwoz force-pushed the dwoz:issue_54755 branch from 0114434 to 602dfa7 Oct 1, 2019
@codecov

This comment has been minimized.

Copy link

codecov bot commented Oct 1, 2019

Codecov Report

Merging #54826 into 2019.2.1 will increase coverage by 31.74%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##           2019.2.1   #54826       +/-   ##
=============================================
+ Coverage      5.02%   36.77%   +31.74%     
=============================================
  Files          1521     1577       +56     
  Lines        253244   269866    +16622     
  Branches      54002    56108     +2106     
=============================================
+ Hits          12734    99238    +86504     
+ Misses       239072   159279    -79793     
- Partials       1438    11349     +9911
Flag Coverage Δ
#centos7 ?
#py2 36.77% <ø> (+31.86%) ⬆️
#py3 ?
#tcp 36.77% <ø> (?)
#ubuntu1604 36.77% <ø> (+31.75%) ⬆️
#zeromq ?
Impacted Files Coverage Δ
salt/proxy/dummy.py 0% <0%> (-80.87%) ⬇️
salt/proxy/docker.py 0% <0%> (-62.5%) ⬇️
salt/proxy/chronos.py 0% <0%> (-50%) ⬇️
salt/proxy/marathon.py 0% <0%> (-50%) ⬇️
salt/transport/frame.py 30.3% <0%> (-46.97%) ⬇️
salt/proxy/cisconso.py 0% <0%> (-46.43%) ⬇️
salt/proxy/arista_pyeapi.py 0% <0%> (-45.66%) ⬇️
salt/proxy/nxos_api.py 0% <0%> (-44.74%) ⬇️
salt/proxy/rest_sample.py 0% <0%> (-38.38%) ⬇️
salt/proxy/netmiko_px.py 0% <0%> (-34.85%) ⬇️
... and 1432 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 433b6fa...602dfa7. Read the comment docs.

1 similar comment
@codecov

This comment has been minimized.

Copy link

codecov bot commented Oct 1, 2019

Codecov Report

Merging #54826 into 2019.2.1 will increase coverage by 31.74%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##           2019.2.1   #54826       +/-   ##
=============================================
+ Coverage      5.02%   36.77%   +31.74%     
=============================================
  Files          1521     1577       +56     
  Lines        253244   269866    +16622     
  Branches      54002    56108     +2106     
=============================================
+ Hits          12734    99238    +86504     
+ Misses       239072   159279    -79793     
- Partials       1438    11349     +9911
Flag Coverage Δ
#centos7 ?
#py2 36.77% <ø> (+31.86%) ⬆️
#py3 ?
#tcp 36.77% <ø> (?)
#ubuntu1604 36.77% <ø> (+31.75%) ⬆️
#zeromq ?
Impacted Files Coverage Δ
salt/proxy/dummy.py 0% <0%> (-80.87%) ⬇️
salt/proxy/docker.py 0% <0%> (-62.5%) ⬇️
salt/proxy/chronos.py 0% <0%> (-50%) ⬇️
salt/proxy/marathon.py 0% <0%> (-50%) ⬇️
salt/transport/frame.py 30.3% <0%> (-46.97%) ⬇️
salt/proxy/cisconso.py 0% <0%> (-46.43%) ⬇️
salt/proxy/arista_pyeapi.py 0% <0%> (-45.66%) ⬇️
salt/proxy/nxos_api.py 0% <0%> (-44.74%) ⬇️
salt/proxy/rest_sample.py 0% <0%> (-38.38%) ⬇️
salt/proxy/netmiko_px.py 0% <0%> (-34.85%) ⬇️
... and 1432 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 433b6fa...602dfa7. Read the comment docs.

@dwoz dwoz force-pushed the dwoz:issue_54755 branch from 602dfa7 to 0bad9cb Oct 1, 2019
@dwoz dwoz merged commit 9e3914a into saltstack:2019.2.1 Oct 1, 2019
17 of 24 checks passed
17 of 24 checks passed
ci/py3/centos7/tcp This commit cannot be built
Details
ci/py2/amazon2 This commit is being built
Details
ci/py2/centos6 This commit is being built
Details
ci/py2/centos7 This commit is being built
Details
ci/py2/centos7/tcp This commit is being built
Details
ci/py2/fedora29 This commit is being built
Details
ci/py3/windows2016 This commit is being built
Details
WIP Ready for review
Details
ci/docs This commit looks good
Details
ci/lint This commit looks good
Details
ci/py2/debian8 This commit looks good
Details
ci/py2/debian9 This commit looks good
Details
ci/py2/ubuntu1604 This commit looks good
Details
ci/py2/ubuntu1604/tcp This commit looks good
Details
ci/py2/ubuntu1804 This commit looks good
Details
ci/py2/windows2016 This commit looks good
Details
ci/py3/amazon2 This commit looks good
Details
ci/py3/centos7 This commit looks good
Details
ci/py3/debian8 This commit looks good
Details
ci/py3/debian9 This commit looks good
Details
ci/py3/fedora29 This commit looks good
Details
ci/py3/ubuntu1604 This commit looks good
Details
ci/py3/ubuntu1604/tcp This commit looks good
Details
ci/py3/ubuntu1804 This commit looks good
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.