napalm-automation-community / napalm-vyos

NAPALM Driver for the VyOS
Apache License 2.0
37 stars 27 forks source link

Update file mode #40

Closed marcushoff closed 3 years ago

marcushoff commented 3 years ago

Default file mode is binary, updated to string

jchristopher327 commented 3 years ago

I'm testing VyOS with Nornir doing config replacements and ran into the following error.

Traceback (most recent call last):
  File "/home/jaychristopher/development/test/venv/lib64/python3.6/site-packages/nornir/core/task.py", line 98, in start
    r = self.task(self, **self.params)
  File "/home/jaychristopher/development/test/venv/lib64/python3.6/site-packages/nornir_napalm/plugins/tasks/napalm_configure.py", line 32, in napalm_configure
    device.load_replace_candidate(filename=filename, config=configuration)
  File "/home/jaychristopher/development/test/venv/lib64/python3.6/site-packages/napalm_vyos/vyos.py", line 130, in load_replace_candidate
    temp_file.write(config)
  File "/usr/lib64/python3.6/tempfile.py", line 485, in func_wrapper
    return func(*args, **kwargs)
TypeError: a bytes-like object is required, not 'str'

I implemented the following locally as a workaround but the changes by @marcushoff also work.

128         if filename is None:
129             temp_file = tempfile.NamedTemporaryFile()
130             temp_file.write(config.encode())
131             temp_file.flush()
132             cfg_filename = temp_file.name
133         else:
134             cfg_filename = filename

@ppieprzycki anything else that's needed to have this merged and a new release?

jchristopher327 commented 3 years ago

My use case is with Nornir+NAPALM to replace the config on the vyos device. The config is generated using Jinja2 templates. The error could be due to this use case so I wanted to test with NAPALM directly by loading the config from a local file (admittedly, there's still a difference on how the config file is generated).

from napalm import get_network_driver
driver = get_network_driver("vyos")
device = driver("vyos-fw", "username", "password")
device.open()
device.load_replace_candidate(filename="vyos.conf")
print(device.compare_config())
device.discard_config()

With the current napalm-vyos release, I was able to run the above commands, which loads a config, and compare it with the running-config. I then made the changes proposed in this PR and didn't get any errors. So I think that this change shouldn't break any existing functionality.