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

Extended NAPALM with PluribusDriver & few more methods #154

Closed
wants to merge 14 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@mirceaulinic
Copy link
Member

mirceaulinic commented Feb 2, 2016

In this PR we propose to extend the capabilities of NAPALM with one more driver for Pluribus devices. Although the library pyPluribus is not yet mature, we are still working on it and will add new features in the near future.
Also we have implemented the following methods for JunOS, IOS-XR, NXOS, EOS and Pluribus:

  • get_arp_table
  • get_mac_table
  • get_ntp_peers
  • get_lldp_neighbors_detailed (which is a detailed version of the existing get_lldp_neighbors -- probably it would be a good idea to merge them in a future release).

Also, on some devices we have designed few other methods such as show_route, get_interfaces_ip or get_bgp_neighbors_detail.

@dbarrosop

This comment has been minimized.

Copy link
Member

dbarrosop commented Feb 2, 2016

Hi!
This is awesome! A couple of things:

  1. Could you document the new methods in base.py? It's important to make sure people understands what the method is supposed to return.
  2. Does pluribus have a VM or something so we can run unittests? Otherwise it would be interesting if at least we could have some mock data to test the results.
  3. Could you provide mock data for the methods you implemented?

Feel free to ask me any doubts you might have.

Regards.
David

@dbarrosop

This comment has been minimized.

Copy link
Member

dbarrosop commented Feb 2, 2016

Could you also break down the PR into smaller PRs? I would suggest one per new method and another one with the new driver. The reasoning is that it's hard to actually peer review a huge PR and some stuff might be merged straight away while other might prompt some discussion and maybe require some changes.

@mirceaulinic

This comment has been minimized.

Copy link
Member

mirceaulinic commented Feb 3, 2016

Hi David,

I will submit soon a couple of PRs as you proposed together with mock inputs. I have not forgot about the documentation and when I will make the requests I will include it as well.

Thanks!
Mircea

@mirceaulinic mirceaulinic deleted the mirceaulinic:CF-NAPALM branch May 13, 2016

dbarrosop added a commit that referenced this pull request Oct 19, 2017

Release 0.20.1 (#155)
* Version bump

* Dbarrosop/fix signatures (#154)

* Fix method signatures

* Use defaults from constants

* Version bump

* Python3 doesnt like relative imports

dbarrosop added a commit that referenced this pull request Oct 19, 2017

Dbarrosop/validate getter args (#175)
* Dbarrosop/fix signatures (#154)

* Fix method signatures

* Use defaults from constants

* Version bump

* Python3 doesnt like relative imports

* Validate now support getters with args

* testing mocked method is in C.ACTION_TYPE_METHODS

* Adding ACTION_TYPE_METHODS to constants file

dbarrosop pushed a commit that referenced this pull request Oct 19, 2017

Merge pull request #154 from mirceaulinic/revert-del
Revert __del__ and move into napalm-base

dbarrosop pushed a commit that referenced this pull request Oct 20, 2017

dbarrosop pushed a commit that referenced this pull request Oct 20, 2017

Fix #153 and optimise the implementation
As described in #154, for newer Junos, it is possible to get the
instance name from the BGP neighbos, so we can enhance the
implementation and reduce the run time by executing a single request
fetching all BGP neighbors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment