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 deprecated warnings #38388

Merged
merged 1 commit into from
Apr 17, 2018
Merged

Fix deprecated warnings #38388

merged 1 commit into from
Apr 17, 2018

Conversation

remyleone
Copy link
Contributor

SUMMARY
ISSUE TYPE
  • Bugfix Pull Request
ANSIBLE VERSION
$ ansible --version
ansible 2.6.0 (fix_deprecated cbdae1b688) last updated 2018/04/06 11:13:21 (GMT +200)
  config file = None
  configured module search path = [u'/Users/sieben/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/sieben/workspace/ansible/lib/ansible
  executable location = /Users/sieben/workspace/ansible/bin/ansible
  python version = 2.7.14 (default, Mar  9 2018, 23:57:12) [GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.39.2)]

@ansibot
Copy link
Contributor

ansibot commented Apr 6, 2018

@ansibot ansibot added bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. deprecated This issue/PR relates to a deprecated module. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. networking Network category support:community This issue/PR relates to code supported by the Ansible community. labels Apr 6, 2018
@mkrizek
Copy link
Contributor

mkrizek commented Apr 6, 2018

But that will explode on python 2.x since base64.encodebytes is not available there , no?

@mkrizek mkrizek removed the needs_triage Needs a first human triage before being processed. label Apr 6, 2018
@remyleone
Copy link
Contributor Author

remyleone commented Apr 6, 2018 via email

@mkrizek
Copy link
Contributor

mkrizek commented Apr 6, 2018

cc @abadger @webknjaz

@abadger
Copy link
Contributor

abadger commented Apr 6, 2018 via email

@webknjaz
Copy link
Member

webknjaz commented Apr 9, 2018

It's also possible to shim it like:

try:
    from base64 import encodebytes
except ImportError:
    from base64 import encodestring as encodebytes

somewhere in compat module and use it from there.

@webknjaz
Copy link
Member

webknjaz commented Apr 9, 2018

Alternatively, add moves to ansible.module_utils._text (to mirror this PR):

import six
for pf in 'de', 'en':
    mv = six.MovedAttribute(
        'base64_%scodebytes' % pf,
        'base64', 'base64',
        '%scodestring' % pf,
        '%scodebytes' % pf
    )
    six.add_move(mv)

from six.moves import base64_encodebytes

@abadger
Copy link
Contributor

abadger commented Apr 9, 2018 via email

@ansibot
Copy link
Contributor

ansibot commented Apr 9, 2018

cc @seuf
click here for bot help

Copy link
Contributor

@seuf seuf left a comment

Choose a reason for hiding this comment

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

LGTM

@remyleone remyleone force-pushed the fix_deprecated branch 2 times, most recently from 0b96b5d to b44786c Compare April 9, 2018 14:31
@remyleone
Copy link
Contributor Author

@abadger done I've changed it to use ansible.module_utils._text import to_bytes tell me what you think :)

@abadger
Copy link
Contributor

abadger commented Apr 9, 2018 via email

@calfonso calfonso added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Apr 11, 2018
@ansibot ansibot removed the community_review In order to be merged, this PR must follow the community review workflow. label Apr 11, 2018
@remyleone
Copy link
Contributor Author

@abadger done

@remyleone
Copy link
Contributor Author

@webknjaz are you satisfied with the current solution?

@mkrizek
Copy link
Contributor

mkrizek commented Apr 17, 2018

@sieben I am curious as to why you didn't use base64.b64encode() as @abadger suggested?

@remyleone
Copy link
Contributor Author

@mkrizek I misread :) Fixed

@abadger abadger merged commit 98fb47b into ansible:devel Apr 17, 2018
@abadger
Copy link
Contributor

abadger commented Apr 17, 2018

Merged to devel and cherry-picked to stable-2.5

@ansible ansible locked and limited conversation to collaborators Apr 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue/PR relates to a bug. deprecated This issue/PR relates to a deprecated module. module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. networking Network category support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants