Initial skeleton for v2 #56
Conversation
…alm-srlinux, adding typing and reimplementing PoC methods for get_bgp_neighbors, get_lldp_neighbors, get_users.
|
CI/CD tests are failing, you can check locally using |
use head when testing the connection in the open method
| def _compose_jsonrpc_url(self): | ||
| """ | ||
| Compose the JSON RPC URL, based on the initialized arguments. | ||
| """ | ||
|
|
||
| proto = ( | ||
| "https" | ||
| if ( | ||
| self.jsonrpc_port == 443 | ||
| or (self.jsonrpc_port != 80 and not self.insecure) | ||
| ) | ||
| else "http" | ||
| ) | ||
| url = f"{proto}://{self.hostname}:{self.jsonrpc_port}/jsonrpc" | ||
| # print(url) | ||
|
|
||
| return url | ||
|
|
||
| def _determine_jsonrpc_port(self, opt_args): | ||
| """ | ||
| Determine the JSON RPC port based on the optional arguments. | ||
| """ | ||
|
|
||
| # by default assume 443 port is used by jsonrpc server | ||
| port: int = 443 | ||
|
|
||
| # if jsonrpc_port is specified, use that | ||
| if opt_args.get("jsonrpc_port"): | ||
| port = opt_args.get("jsonrpc_port") | ||
| # when insecure is set and jsonrpc_port is not specified, use 80 | ||
| elif opt_args.get("insecure"): | ||
| port = 80 | ||
|
|
||
| return port |
There was a problem hiding this comment.
I added these two helper functions to have some flexibility and testability on how we determine the jsonrpc port and the resulting URL
There was a problem hiding this comment.
Excellent. There’s a few other methods throughout the place that I think could do with the same treatment. These methods are also a good place to start for unit tests.
There was a problem hiding this comment.
also added a dumb unit test (which fails for now since the driver wants to have some cert files which we don't have yet in the mock data)
Thanks mate, we’ll probably review the tests and look to improve them. We want to make the driver a bit more friendly to stub and test in general. |
There was a problem hiding this comment.
@hellt We probably don’t have to reinvent the wheel here, these types are already in napalm.base. https://github.com/napalm-automation/napalm/blob/develop/napalm/base/models.py#L25
yes, we can use the upstream types (and extend them when needed) |
Major simplifications and new abstractions, renamed napalm-srl --> napalm-srlinux, adding typing and reimplementing PoC methods for get_bgp_neighbors, get_lldp_neighbors, get_users.