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

Add eos_fn0039_config optional arg to toggle FN 0039 on config commands #1212

Merged
merged 3 commits into from
May 15, 2020
Merged

Add eos_fn0039_config optional arg to toggle FN 0039 on config commands #1212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f0626b7
Add eos_fn0039_config optional arg to toggle FN 0039 on config commands
mirceaulinic May 13, 2020
1147048
Merge branch 'develop' into fn-0039-opt-arg
ktbyers May 14, 2020
f8d85e5
Merge branch 'develop' into fn-0039-opt-arg
mirceaulinic May 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/support/index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,13 @@ ____________________________________
* :code:`transport` (eos, ios, nxos) - Protocol to connect with (see `The transport argument`_ for more information).
* :code:`use_keys` (ios, iosxr, nxos_ssh) - Paramiko argument, enable searching for discoverable private key files in ``~/.ssh/`` (default: ``False``).
* :code:`eos_autoComplete` (eos) - Allows to set `autoComplete` when running commands. (default: ``None`` equivalent to ``False``)
* :code:`eos_fn0039_config` (eos) - Transform old style configuration to the new
style, available beginning with EOS release 4.23.0, as per FN 0039. Beware
that enabling this option will change the configuration you're loading
through NAPALM. Default: ``False`` (won't change your configuration
commands).

.. versionadded:: 3.0.1

The transport argument
______________________
Expand Down
9 changes: 7 additions & 2 deletions napalm/eos/eos.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ def _process_optional_args(self, optional_args):
transport = optional_args.get(
"transport", optional_args.get("eos_transport", "https")
)
self.fn0039_config = optional_args.pop("eos_fn0039_config", False)
try:
self.transport_class = pyeapi.client.TRANSPORTS[transport]
except KeyError:
Expand Down Expand Up @@ -285,9 +286,13 @@ def _load_config(self, filename=None, config=None, replace=True):

try:
if self.eos_autoComplete is not None:
self.device.run_commands(commands, autoComplete=self.eos_autoComplete)
self.device.run_commands(
commands,
autoComplete=self.eos_autoComplete,
fn0039_transform=self.fn0039_config,
)
else:
self.device.run_commands(commands)
self.device.run_commands(commands, fn0039_transform=self.fn0039_config)
except pyeapi.eapilib.CommandError as e:
self.discard_config()
msg = str(e)
Expand Down
12 changes: 7 additions & 5 deletions napalm/eos/pyeapi_syntax_wrapper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@ def run_commands(self, commands, **kwargs):
:param kwargs: other args
:return: list of outputs
"""
if isinstance(commands, str):
new_commands = [cli_convert(commands, self.cli_version)]
else:
new_commands = [cli_convert(cmd, self.cli_version) for cmd in commands]
fn0039_transform = kwargs.pop("fn0039_transform", True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this default to False if fn0039_transform doesn't exist?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I thought I'd default this to True as we may want to disable this behaviour only for the config-related methods (the getters would always convert the commands under the hood)... so it was mostly a matter of whether we update every getter (everywhere run_commands is called) or just _load_config. Not sure if I'm explaining this well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, that makes sense.

So internal to NAPALM for show commands, we would do this automatic conversion. But then for any configuration changes by end users, they would default to no conversion (with an option they could specify that would do the conversion).

Is that the correct understanding?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, correct, much better worded like this. 😄

if fn0039_transform:
if isinstance(commands, str):
commands = [cli_convert(commands, self.cli_version)]
else:
commands = [cli_convert(cmd, self.cli_version) for cmd in commands]

return super(Node, self).run_commands(new_commands, **kwargs)
return super(Node, self).run_commands(commands, **kwargs)
12 changes: 9 additions & 3 deletions test/eos/test_heredoc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ def test_heredoc(self):
"idle-timeout 15",
]

self.device.device.run_commands.assert_called_with(expected_result)
self.device.device.run_commands.assert_called_with(
expected_result, fn0039_transform=False
)

def test_mode_comment(self):
raw_config = dedent(
Expand Down Expand Up @@ -108,7 +110,9 @@ def test_mode_comment(self):
"permit host 192.0.2.3",
]

self.device.device.run_commands.assert_called_with(expected_result)
self.device.device.run_commands.assert_called_with(
expected_result, fn0039_transform=False
)

def test_heredoc_with_bangs(self):

Expand Down Expand Up @@ -145,4 +149,6 @@ def test_heredoc_with_bangs(self):
"idle-timeout 15",
]

self.device.device.run_commands.assert_called_with(expected_result)
self.device.device.run_commands.assert_called_with(
expected_result, fn0039_transform=False
)