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.

Sign up for GitHub

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

Jump to bottom

Adding enos_command module and unit test #32782

Merged
merged 8 commits into from
Nov 14, 2017
Merged

Adding enos_command module and unit test #32782

merged 8 commits into from
Nov 14, 2017

Conversation

amuraleedhar
Copy link
Contributor

@amuraleedhar amuraleedhar commented Nov 10, 2017 •
edited

SUMMARY

Creating the command module for enos from Lenovo.

ISSUE TYPE

New Module Pull Request

COMPONENT NAME

lib/ansible/modules/network/enos/enos_command.py
test/units/modules/network/enos/test_enos_command.py

ANSIBLE VERSION

ansible 2.5.0
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 = /usr/local/lib/python2.7/dist-packages/ansible-2.5.0-py2.7.egg/ansible
executable location = /usr/local/bin/ansible
python version = 2.7.6 (default, Oct 26 2016, 20:30:19) [GCC 4.8.4]

ADDITIONAL INFORMATION

This is part of an effort to create modules for enos based switches from Lenovo.

@ansibot
Copy link
Contributor

ansibot commented Nov 10, 2017

cc @Qalthos @ganeshrn @gundalow @kedarX @privateip @rcarrillocruz @trishnaguha
click here for bot help

@ansibot ansibot added affects_2.5 This issue/PR affects Ansible v2.5 community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. networking Network category new_module This PR includes a new module. new_plugin This PR includes a new plugin. support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests. labels Nov 10, 2017
@mkrizek mkrizek removed the needs_triage Needs a first human triage before being processed. label Nov 10, 2017
@amuraleedhar amuraleedhar mentioned this pull request Nov 10, 2017
Closed
@amuraleedhar
Copy link
Contributor Author

Please review these and merge it .
Thanks
-Anil

@ansibot
Copy link
Contributor

ansibot commented Nov 10, 2017

@amuraleedhar this PR contains more than one new module.

Please submit only one new module per pullrequest. For further explanation, please read grouped module documentation

click here for bot help

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. community_review In order to be merged, this PR must follow the community review workflow. and removed community_review In order to be merged, this PR must follow the community review workflow. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Nov 10, 2017
@amuraleedhar amuraleedhar changed the title Adding git_command module and its UT file Adding enos_command module and its UT file Nov 10, 2017
@amuraleedhar
Copy link
Contributor Author

amuraleedhar commented Nov 10, 2017 •
edited

Please review the enos_command module and merge it .
Thanks
-Anil

ganeshrn
ganeshrn reviewed Nov 11, 2017
View reviewed changes
lib/ansible/modules/network/enos/enos_command.py Outdated
password: admin
timeout: 30
authorize: True
auth_pass:
Copy link
Member

Choose a reason for hiding this comment

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

Value is missing here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing auth_pass as its not required by default.

ganeshrn
ganeshrn reviewed Nov 11, 2017
View reviewed changes
lib/ansible/modules/network/enos/enos_command.py Outdated
module = AnsibleModule(argument_spec=spec, supports_check_mode=True)

warnings = list()
check_args(module, warnings)
Copy link
Member

Choose a reason for hiding this comment

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

Is check_args call required, as the function is empty? Fo other network modules it is present due to legacy reasons which need to removed in respective platform modules refactor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will remove that

ganeshrn
ganeshrn approved these changes Nov 14, 2017
View reviewed changes
Copy link
Member

@ganeshrn ganeshrn left a comment

Choose a reason for hiding this comment

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

lgtm

@gundalow gundalow changed the title Adding enos_command module and its UT file Adding enos_command module and unit test Nov 14, 2017
gundalow
gundalow requested changes Nov 14, 2017
View reviewed changes
Copy link
Contributor

@gundalow gundalow left a comment

Choose a reason for hiding this comment

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

Just a few minor comments

lib/ansible/modules/network/enos/enos_command.py Outdated
See examples.
required: false
default: null
aliases: ['waitfor']
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this line. This is only in old modules for backwards compatibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

lib/ansible/modules/network/enos/enos_command.py Outdated
# { command: <str>, prompt: <str>, response: <str> }
commands=dict(type='list', required=True),

wait_for=dict(type='list', aliases=['waitfor']),
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove , aliases=['waitfor']

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

lib/ansible/modules/network/enos/enos_command.py Outdated
@@ -0,0 +1,220 @@
#!/usr/bin/python
#
# Copyright: Ansible Project
Copy link
Contributor

Choose a reason for hiding this comment

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

Lenovo copyright?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes Changing it.

test/units/modules/network/enos/test_enos_command.py Outdated
@@ -0,0 +1,103 @@
# (c) 2016 Red Hat Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

2017
Lenovo copyright?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, Changing it

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Nov 14, 2017
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Nov 14, 2017
@gundalow gundalow merged commit 177a4fb into ansible:devel Nov 14, 2017
@gundalow
Copy link
Contributor

Merged into devel will be released in 2.5.0

@amuraleedhar amuraleedhar deleted the enos-module-command branch August 9, 2018 11:32
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gundalow gundalow gundalow approved these changes

@ganeshrn ganeshrn ganeshrn approved these changes

Assignees
No one assigned
Labels
affects_2.5 This issue/PR affects Ansible v2.5 community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. networking Network category new_module This PR includes a new module. new_plugin This PR includes a new plugin. support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@amuraleedhar @ansibot @gundalow @ganeshrn @mkrizek