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

Allow network_cli type for platforms using persistent connection #120

Merged
merged 7 commits into from
Oct 15, 2020

Conversation

ganeshrn
Copy link
Member

SUMMARY

Fixes:
#119

  • For platforms using persistent allow network_cli connection type
    In ansible 2.6 network_cli connection was added as first class
    connection, however some of the community platform code was not
    updated to use network_cli.
  • Deprecate local connection type for platforms that use persistent
    connection framework and have cliconf and terminal plugins
    implemented.
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

plugins/action/ce.py
plugins/action/cnos.py
plugins/action/enos.py
plugins/action/ironware.py
plugins/action/sros.py
plugins/module_utils/network/aireos/aireos.py
plugins/module_utils/network/aruba/aruba.py
plugins/module_utils/network/cloudengine/ce.py
plugins/module_utils/network/cnos/cnos.py
plugins/module_utils/network/enos/enos.py
plugins/module_utils/network/ironware/ironware.py

ADDITIONAL INFORMATION

Fixes:
ansible-collections#119

*  For platforms using persistent allow network_cli connection type
   In ansible 2.6 `network_cli` connection was added as first class
   connection, however some of the community platform code was not
   updated to use `network_cli`.
*  Deprecate local connection type for platforms that use persistent
   connection framework and have cliconf and terminal plugins
   implemented.
changelogs/fragments/119_fix_connection_check_issue.yml Outdated Show resolved Hide resolved
changelogs/fragments/119_fix_connection_check_issue.yml Outdated Show resolved Hide resolved
plugins/action/ce.py Outdated Show resolved Hide resolved
task_vars['ansible_socket'] = socket_path
msg = "connection local support for this module is deprecated use either" \
" 'network_cli' or 'ansible.netcommon.network_cli' connection"
deprecate(msg, version='4.0.0', collection_name='community.network')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this method has any effect (it stores its result in global variables which aren't read). Better use display.deprecated().

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Cannot judge all the code changes, but the things I can judge look good. If this is merged by tomorrow, it will definitely be included in community.network 1.2.0.

@felixfontein felixfontein added the check-before-release PR will be looked at again shortly before release and merged if possible. label Sep 29, 2020
@ganeshrn
Copy link
Member Author

@felixfontein Thanks for reviewing the PR. It will be great if we can get some reviews from the individual platform owners. For platform using persistent connection framework, we would like to get away from using connection=local and promote the use of connection=network_cli.
Can we add the PR to community meeting agenda if it can help get more eyes?

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Sep 29, 2020
@felixfontein
Copy link
Collaborator

@ganeshrn feel free to add it to the agenda. Not sure whether it will get a lot of additional eyes, I'm not sure anyone interested in this collection attends that meeting (except @gundalow and me). It might make more sense to CC all relevant people in this PR. (Once we have ansibullbot here it should have already done it, but we don't have the bot yet...)

@ganeshrn
Copy link
Member Author

ganeshrn commented Sep 29, 2020

@mihaibroc
Copy link

The changes (for Lenovo CNOS and ENOS) look good for me.
Thank you,
Mihai

@ganeshrn ganeshrn merged commit b5629d7 into ansible-collections:main Oct 15, 2020
patchback bot pushed a commit that referenced this pull request Oct 15, 2020
* Allow network_cli type for platforms using persistent connection

Fixes:
#119

*  For platforms using persistent allow network_cli connection type
   In ansible 2.6 `network_cli` connection was added as first class
   connection, however some of the community platform code was not
   updated to use `network_cli`.
*  Deprecate local connection type for platforms that use persistent
   connection framework and have cliconf and terminal plugins
   implemented.

* add changelog

* Fix review comments

* minor updates

* remove unwanted space

* FIx more santiy issues

* fix review comments

(cherry picked from commit b5629d7)
felixfontein pushed a commit that referenced this pull request Oct 15, 2020
… (#133)

* Allow network_cli type for platforms using persistent connection

Fixes:
#119

*  For platforms using persistent allow network_cli connection type
   In ansible 2.6 `network_cli` connection was added as first class
   connection, however some of the community platform code was not
   updated to use `network_cli`.
*  Deprecate local connection type for platforms that use persistent
   connection framework and have cliconf and terminal plugins
   implemented.

* add changelog

* Fix review comments

* minor updates

* remove unwanted space

* FIx more santiy issues

* fix review comments

(cherry picked from commit b5629d7)

Co-authored-by: Ganesh Nalawade <[email protected]>
@felixfontein
Copy link
Collaborator

@ganeshrn thanks for implementing this!
@mihaibroc thanks for reviewing!

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