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 "save_when" #49613

Open
wants to merge 6 commits into
base: devel
from

Conversation

Projects
None yet
3 participants
@FedericoOlivieri

FedericoOlivieri commented Dec 6, 2018

I have add "save_when" functionality, taken from ios_config. However ios_banner arguments need to be updated. I have tried to reverse engineering the code to understand where the argument list has to be updated but I could not find the bottom of it

SUMMARY
ISSUE TYPE
  • Bugfix Pull Request
  • Docs Pull Request
  • Feature Pull Request
  • New Module Pull Request
COMPONENT NAME
ADDITIONAL INFORMATION

add "save_when"
I have add "save_when" functionality, taken from ios_config. However ios_banner arguments need to be updated. I have tried to reverse engineering the code to understand where the argument list has to be updated but I could not find the bottom of it
@ansibot

This comment has been minimized.

Contributor

ansibot commented Dec 6, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Dec 6, 2018

@FedericoOlivieri, just so you are aware we have a dedicated Working Group for network.
You can find other people interested in this in #ansible-network on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

@ansibot

This comment has been minimized.

Contributor

ansibot commented Dec 6, 2018

The test ansible-test sanity --test pylint [explain] failed with 2 errors:

lib/ansible/modules/network/ios/ios_banner.py:180:82: undefined-variable Undefined variable 'diff_ignore_lines'
lib/ansible/modules/network/ios/ios_banner.py:181:82: undefined-variable Undefined variable 'diff_ignore_lines'

The test ansible-test sanity --test pep8 [explain] failed with 3 errors:

lib/ansible/modules/network/ios/ios_banner.py:148:1: E302 expected 2 blank lines, found 1
lib/ansible/modules/network/ios/ios_banner.py:157:1: E302 expected 2 blank lines, found 1
lib/ansible/modules/network/ios/ios_banner.py:164:9: E128 continuation line under-indented for visual indent

click here for bot help

@ansibot ansibot added needs_revision and removed core_review labels Dec 6, 2018

@FragmentedPacket

This comment has been minimized.

Contributor

FragmentedPacket commented Dec 7, 2018

if module.params['save_when'] == 'always' or module.params['save']:
save_config(module, result)
elif module.params['save_when'] == 'modified':
output = run_commands(module, ['show running-config', 'show startup-config'])
running_config = NetworkConfig(indent=1, contents=output[0], ignore_lines=diff_ignore_lines)
startup_config = NetworkConfig(indent=1, contents=output[1], ignore_lines=diff_ignore_lines)
if running_config.sha1 != startup_config.sha1:
save_config(module, result)
elif module.params['save_when'] == 'changed' and result['changed']:
save_config(module, result)

I'd make a few changes:

  • I would move the lines above (L175-186) to below L204 with the correct indentation.
  • Remove ignore_lines=diff_ignore_lines from L180/181
  • Remove the extra ) from L164

Also, what errors are you receiving when running this modified module?

@FedericoOlivieri

This comment has been minimized.

FedericoOlivieri commented Dec 7, 2018

The error I get is:

FAILED! => {"changed": false, "msg": "Unsupported parameters for (ios_banner) module: save_when Supported parameters include: auth_pass, authorize, banner, host, password, port, provider, ssh_keyfile, state, text, timeout, username"}

FedericoOlivieri added some commits Dec 7, 2018

Update ios_banner
Move the lines above (L175-186) to below L204 with the correct indentation.
Remove ignore_lines=diff_ignore_lines from L180/181
Remove the extra ) from L164
@ansibot

This comment has been minimized.

Contributor

ansibot commented Dec 7, 2018

The test ansible-test sanity --test pep8 [explain] failed with 6 errors:

lib/ansible/modules/network/ios/ios_banner.py:107:1: E302 expected 2 blank lines, found 0
lib/ansible/modules/network/ios/ios_banner.py:121:1: E302 expected 2 blank lines, found 0
lib/ansible/modules/network/ios/ios_banner.py:138:1: E302 expected 2 blank lines, found 0
lib/ansible/modules/network/ios/ios_banner.py:147:1: E302 expected 2 blank lines, found 0
lib/ansible/modules/network/ios/ios_banner.py:155:1: E302 expected 2 blank lines, found 0
lib/ansible/modules/network/ios/ios_banner.py:196:1: E305 expected 2 blank lines after class or function definition, found 0

click here for bot help

@ansibot ansibot added the ci_verified label Dec 7, 2018

fix space
ib/ansible/modules/network/ios/ios_banner.py:107:1: E302 expected 2 blank lines, found 0
lib/ansible/modules/network/ios/ios_banner.py:121:1: E302 expected 2 blank lines, found 0
lib/ansible/modules/network/ios/ios_banner.py:138:1: E302 expected 2 blank lines, found 0
lib/ansible/modules/network/ios/ios_banner.py:147:1: E302 expected 2 blank lines, found 0
lib/ansible/modules/network/ios/ios_banner.py:155:1: E302 expected 2 blank lines, found 0
lib/ansible/modules/network/ios/ios_banner.py:196:1: E305 expected 2 blank lines after class or function definition, found 0

@ansibot ansibot removed the ci_verified label Dec 7, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Dec 7, 2018

The test ansible-test sanity --test pylint [explain] failed with 9 errors:

lib/ansible/modules/network/ios/ios_banner.py:124:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/network/ios/ios_banner.py:142:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/network/ios/ios_banner.py:143:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/network/ios/ios_banner.py:153:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/network/ios/ios_banner.py:154:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/network/ios/ios_banner.py:163:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/network/ios/ios_banner.py:164:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/network/ios/ios_banner.py:206:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/network/ios/ios_banner.py:207:0: trailing-whitespace Trailing whitespace

The test ansible-test sanity --test pep8 [explain] failed with 9 errors:

lib/ansible/modules/network/ios/ios_banner.py:124:1: W293 blank line contains whitespace
lib/ansible/modules/network/ios/ios_banner.py:142:1: W293 blank line contains whitespace
lib/ansible/modules/network/ios/ios_banner.py:143:1: W293 blank line contains whitespace
lib/ansible/modules/network/ios/ios_banner.py:153:1: W293 blank line contains whitespace
lib/ansible/modules/network/ios/ios_banner.py:154:1: W293 blank line contains whitespace
lib/ansible/modules/network/ios/ios_banner.py:163:1: W293 blank line contains whitespace
lib/ansible/modules/network/ios/ios_banner.py:164:1: W293 blank line contains whitespace
lib/ansible/modules/network/ios/ios_banner.py:206:1: W293 blank line contains whitespace
lib/ansible/modules/network/ios/ios_banner.py:207:1: W293 blank line contains whitespace

click here for bot help

@ansibot ansibot added the ci_verified label Dec 7, 2018

@ansibot ansibot added stale_ci and removed needs_ci stale_ci labels Dec 8, 2018

@Qalthos Qalthos requested a review from gdpak Dec 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment