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

Add deprecation warning path for glance state and execution module #55592

Merged
merged 2 commits into from
Dec 20, 2019

Conversation

Ch3LL
Copy link
Contributor

@Ch3LL Ch3LL commented Dec 10, 2019

What does this PR do?

Add deprecation warning path for glance state and execution module

What issues does this PR fix or reference?

#49414

Previous Behavior

glance module was set to be deprecated in the neon release

New Behavior

glance module will not be deprecated till the Aluminium release. This is because the glance state module which uses the glance module was never set on a deprecation path. So both the glance execution and state modules are set for deprecation in Aluminium.

Tests written?

No - adding deprecation warning

Commits signed with GPG?

Yes

@Ch3LL Ch3LL requested a review from a team as a code owner December 10, 2019 19:44
@ghost ghost requested a review from xeacott December 10, 2019 19:44
@Ch3LL Ch3LL changed the title Deprecate glance module Add deprecation warning path for glance state and execution module Dec 10, 2019
@@ -104,7 +104,7 @@ def _auth(profile=None, api_version=2, **connection_args):
Only intended to be used within glance-enabled modules
'''
__utils__['versions.warn_until'](
Copy link
Collaborator

@s0undt3ch s0undt3ch Dec 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is not your change but why are we preferring __utils__ instead of the direct function call?
You know the reason behind that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I thought we always preferred __utils__ where possible since its packed in via the loader instead of importing directly. Is there a reason not to?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess there really isn't a reason, I apparently just dislike it's usage, seems like mixing loader stuff with regular utils modules, but it's just my personal preference.

@codecov
Copy link

codecov bot commented Dec 17, 2019

Codecov Report

Merging #55592 into master will decrease coverage by 1.42%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #55592      +/-   ##
==========================================
- Coverage   39.58%   38.17%   -1.41%     
==========================================
  Files        1484     1463      -21     
  Lines      265124   259754    -5370     
  Branches    57299    54886    -2413     
==========================================
- Hits       104924    99134    -5790     
- Misses     148015   149368    +1353     
+ Partials    12185    11252     -933
Flag Coverage Δ
#amazon1 38.17% <ø> (?)
#archlts ?
#centos7 ?
#cloud ?
#proxy ?
#py2 38.17% <ø> (-1.28%) ⬇️
#py3 ?
#runtests 38.17% <ø> (-1.41%) ⬇️
#ubuntu1604 ?
#zeromq 38.17% <ø> (-0.48%) ⬇️
Impacted Files Coverage Δ
salt/states/acme.py 20% <0%> (-76.77%) ⬇️
salt/modules/zcbuildout.py 15.42% <0%> (-62%) ⬇️
salt/transport/frame.py 27.28% <0%> (-56.06%) ⬇️
salt/states/docker_network.py 9.13% <0%> (-51.45%) ⬇️
salt/states/zookeeper.py 8.53% <0%> (-50.38%) ⬇️
salt/modules/zookeeper.py 25.93% <0%> (-46.91%) ⬇️
salt/states/docker_container.py 5.32% <0%> (-46.06%) ⬇️
salt/utils/smb.py 19.84% <0%> (-40.48%) ⬇️
salt/modules/acme.py 18.75% <0%> (-39.92%) ⬇️
salt/states/zcbuildout.py 32.76% <0%> (-37.93%) ⬇️
... and 281 more

@dwoz dwoz merged commit 8313598 into saltstack:master Dec 20, 2019
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