Refactoring code to adhere to persistence connection.

[amuraleedhar]

SUMMARY

In this PR I am moving the code which is in util file cnos.py to respective module code. There was a review comment is this regard that util file is of huge size. This is an effort to reduce the size.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

lib\ansible\modules\network\cnos\cnos_rollback.py
lib\ansible\module_util\network\cnos\cnos.py

ANSIBLE VERSION

ansible 2.7.0.dev0 (devel f9cbdcd) last updated 2018/07/03 14:55:43 (GMT +550)
config file = /etc/ansible/ansible.cfg
configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
ansible python module location = /home/ansible/sheru/ansible/lib/ansible
executable location = /home/ansible/sheru/ansible/bin/ansible
python version = 2.7.6 (default, Nov 23 2017, 15:49:48) [GCC 4.8.4]

ADDITIONAL INFORMATION

Unused code in cnos.py is removed. Tested with CNOS Mars switch for SFTP and SCP

[ansibot]

cc @Qalthos @dkasberg @ganeshrn @gdpak @rcarrillocruz @trishnaguha
click here for bot help

[ansibot]

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/network/cnos/cnos_rollback.py:304:0: trailing-newlines Trailing newlines

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/modules/network/cnos/cnos_rollback.py:304:1: W391 blank line at end of file

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/network/cnos/cnos_rollback.py:303:0: E109 Next to last line should be: if __name__ == "__main__":

click here for bot help

[sivel]

It appears this code was originally (in it's refactored form) licensed as BSD. I believe you will need to either include license information for this function stating that this function is still BSD licensed, or get approval from the prior authors of the removed code indicated above, to relicense as GPLv3.

[amuraleedhar]

I am the only author for the entire solutions for supporting CNOS and ENOS switches on behalf of Lenovo. There has been a review comment from community that the utility file is growing in size which is further affecting its maintainability. So it has been decided to move few util code to module file.

[amuraleedhar]

Please suggest me, what should I do in this regard.

[sivel]

I can see attribution for 2 users, across the code you are removing:

dkasberg - dbb4521
amuraleedhar - ffd701c

I'm an unsure as to whether you are permitted to make a relicensing decision on behalf of dkasberg as a Lenovo employee.

[amuraleedhar]

David Kasberg has retired from Lenovo and is no more in world of coding. We are still very much in contact. Should I contact him for approval?.

[sivel]

cc @gundalow

[amuraleedhar]

Hi Sivel,
Lenovo Legal team is suggesting to embed BSD License to the snippet that is getting moved from cnos.py. Once OSC team approves the sample code modification I have done, I will make necessary changes here for your approval. May be by Tuesday.

[gundalow]

@amuraleedhar That sounds good. Can you please get this PR updated do we can merge before community freeze on 30th?

[ansibot]

cc @justjais
click here for bot help

[Qalthos]

The licensing issue aside, this looks fine as far as the refactor is concerned.

[Qalthos]

If you like, something like this can be redone as if protocol in ('tftp', 'ftp', 'sftp', 'scp'):

[amuraleedhar]

Sure, I will change that. I have requested for a mail from legal department. They are yet to respond. Will do this in meantime

[amuraleedhar]

Please review and merge it now. Regards

-Anil From: John R Barker <notifications@github.com> Sent: Tuesday, August 28, 2018 7:13 PM To: ansible/ansible <ansible@noreply.github.com> Cc: Anil Kumar Muraleedharan <amuraleedhar@lenovo.com>; Mention <mention@noreply.github.com> Subject: [External] Re: [ansible/ansible] Refactoring code to adhere to persistence connection. (#44398) @gundalow commented on this pull request.

________________________________ In lib/ansible/modules/network/cnos/cnos_rollback.py<#44398 (comment)>:

@@ -183,6 +190,70 @@

from collections import defaultdict +# Utility Method to rollback the running config or start up copnfig +# This method supports only SCP or SFTP or FTP or TFTP +def doConfigRollBack(module, prompt, answer): @amuraleedhar<https://github.com/amuraleedhar> That sounds good. Can you please get this PR updated do we can merge before community freeze on 30th? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#44398 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ANdCcosHu2SQZVV5cByW42kDUhN9pOM6ks5uVUj0gaJpZM4WECIf>.

[Qalthos]

Included for posterity, here are the relevant bits of the response from Lenovo legal forwarded to me:

As to the specific changes identified in PR #44398, based on the review of the newly contributed code to “cnos_rollback.py” file vs. the code currently found with the “cnos.py” file, it is our understanding that, contrary to Anil’s description submitted in the PR, outside of the functional similarity that might exist between both code fragments, there is nothing else in common. Specifically, the code Anil would like to contribute to the “cnos_rollback.py” file is completely different from the code found in the “cnos.py” file. In other words, this is newly written code by Anil.

Furthermore, the work contributed by Anil to the “cnos_rollback.py” under the GPLv3 license and copyrighted to Lenovo, does not represent either directly or in modified form the original copyright material contributed by Dave Kasberg and found in the cnos.py file.

Hence, I would greatly appreciate you accepting/recognizing Anil as the future gatekeeper and contributor to the Ansible project from Lenovo. And, I would also ask, if you can accept Anil’s contributions/changes made to both cnos.py and cnos_rollback.py files.