With the repo being called napalm-srlinux, it is confusing to have the driver be called 'napalm-srl'

Consider https://www.python.org/dev/peps/pep-0423/#how-to-rename-a-project

Based on napalm documentation the compare_config() should be returning a string - https://napalm.readthedocs.io/en/latest/base.html#napalm.base.base.NetworkDriver.compare_config

Current compare_config implementation for JSON returns list of dictionaries. Also that's missing any formatting so it's not easy for human to review.
Also the current format/data type is not consistent with the return values from compare_config() for on-box set-style diff - which is nicely formatted human readable string.

The simplest fix might be to return pprint.pprint(<diff>) from the JSON diff. With exception of "no diff' for which I think napalm should be returning empty string.
Best fix I guess might be some form of unified-diff style output - lines with '+' prefix for added parts, '-' line prefix for removed parts.

Because the user:password is included in the URL, it shows up in (error) logs

Suggestion is to replace with HTTP Basic Authentication using a header instead

Hi,
I have a 7220 D3 box with v21.6.4 SW and the latest napalm-srlinux module.
I'm trying to fetch lldp neighbors, but it returns empty dict, although there are a few neighbors visible from the CLI and even via gnmic.

get_facts works fine, so it doesn't seem like a coding issue.

Code:
`

driver = get_network_driver("srl")

srl_args = {
    "gnmi_port": 57400,
    "jsonrpc_port": 443,
    "target_name": "<addr>",
    "tls_cert":f"{certs_path}/client.pem",
    "tls_ca": f"{certs_path}/ca.pem",
    "tls_key": f"{certs_path}/client-key.pem",
    "skip_verify": True,
    "insecure": True,
    "encoding": "JSON_IETF"
} 

device = driver("<addr>", user, pass, 60, srl_args)
device.open()
print("LLDP:", device.get_lldp_neighbors())
device.close()

`

I'm using this driver to get running config (get_config() function) and then later I'm using the obtained config file to check that nothing changed on the device (load_replace_candidate() and compare_config() functions).

Despite the diff should be empty, the compare_config returns quite long diff.

So it seems the variable skip_verify variable is redundant since the insecure connection part of the code is commented out.
https://github.com/napalm-automation-community/napalm-srlinux/blob/main/napalm_srl/srl.py#L2502

Any specific reason for that ?

The generated protobuf files are getting old, which may lead to issues: #17

It would be better to rewrite the plugin to use pyGnmi for gNMI protocol interactions, such that we can lift along with updated versions as they come out

I built a Netbox container with support for both the SROS and SRLinux NAPALM drivers. It nearly doubled in size, compared to the original (728MB versus 446MB originally)

It may make more sense to also use Netconf towards SRLinux, perhaps even using a single harmonized driver.

Current behaviour of the driver is that it catches the exceptions and uses "stdout" print for saying what went wrong, but it's not re-raising the exceptions.
One example of how this handling provides "weird" output is when doing following operations with incorrect credentials

with driver(<args>) as device:
  result = device.get_config(retrieve='running')
  config = result['running']

This code doesn't fail on the open() nor the get_config() calls.

>>> result['running']
...
TypeError: 'NoneType' object is not subscriptable

So this makes the debugging harder also I would say it's quite non-standard.

The other thing is the logging. The prints statements are hard to integrate into other applications, and the driver can also be used with other than CLI programs. So it's not possible to make the prints integrated with logging files for long-running processes/service or with centralised logging systems.

My suggestion is to replace the prints statements with logging module and to reraise the exceptions (either the same or based on napalm library exceptions). Happy to hear other opinions on the logging and exceptions handling problem described above.

Ansible 2.9.12 is quite old, is there any reason why requiring this version ? - https://github.com/napalm-automation-community/napalm-srlinux/blob/main/requirements.txt#L2

Also what's the reason for raising AnsibleConnectionFailure when failed to read certificate files? - https://github.com/napalm-automation-community/napalm-srlinux/blob/main/napalm_srl/srl.py#L2552
This is a napalm driver so I don't think it needs to be used together with ansible, so I don't understand why ansible exception. Napalm has its own set of exceptions so why not to use that? or a python standard exception?

@jbemmel I'm happy to raise a PR for this, just wants to know if it's fine to remove the ansible requirement.

It is only used to raise a connection exception - no need to pull in full Ansible package for that...

From the mapping documentation https://github.com/napalm-automation-community/napalm-srlinux/blob/main/Summary_of_Methods.pdf in section 2.6 about load_replace_config it says the follwoing:

Limitation
This method has implicit commit, when using this command user needs to be careful since the candidate config will be merged with running config along without an explicit commit. Numeric value for the position in the list of the change, and not the ID of the object. The ID of the object is stored as a separate leaf object in the SRL OS Yang models.

When going through the code https://github.com/napalm-automation-community/napalm-srlinux/blob/main/napalm_srl/srl.py#L2346 it doesn't suggest that there is any commit happening.

Can you please clarify what is the default behaviour here.

CI checks upon code changes - see napalm-sros driver

Up to and including Python 3.6, 'strptime' has an issue with time zone parsing. Suggest to use python-dateutil (https://dateutil.readthedocs.io/en/stable/) instead

Is there a way to update startup config using this napalm driver? So in case the device reboots, the config stays the same
If this is not possible with current implementation, could this please be added?

I believe for example with Juniper napalm driver, the startup config gets updated when commit_config() operation is done.