search

scaleway / netbox-netprod-importer

Poll data from network devices in production and import it into netbox
GNU General Public License v3.0
104 stars 32 forks source link

Correction of errors and adding functionality #15

Closed VictorPavlushin closed 5 years ago

VictorPavlushin commented 5 years ago

1) Fixed cable related errors 2) Added support for SDP for nexus 3) Added correct handling of cli via https for nxos driver

VictorPavlushin commented 5 years ago

I added another cdp support for ios: commit

aruhier commented 5 years ago

I tested on a nexus 9000, it's working great, thanks a lot!

As you added a new feature a few days ago, do you want me to merge this PR or it's still "work in progress"?

aruhier commented 5 years ago

Also, could you just add about the discovery_protocol attributes in the interconnect doc please? You added it in the quickstart, which is great, but it would also make sense here.

If you don't have any time to do it, I will add it after merging this, no problem ;)

Thanks a lot!

VictorPavlushin commented 5 years ago

Yes, you can commit. I added the documentation, but my English is far from perfect ;)

Could you send a sample output REST get-lldp-neighbors-information for junos, for writing the autotest. I have access only EX series: https://forums.juniper.net/t5/Junos/REST-API-for-EX-series/td-p/318479

aruhier commented 5 years ago

Hi @VictorPavlushin, I kind of merge your PR, thanks a lot. I changed a few things, dropped some repeated commits due to a merge on your branch, etc.

Please, for next time, prefer doing multiple pull requests over multiple branches on your side, it will be way easier on my side to merge and avoid to change anything ;)

aruhier commented 5 years ago

However, I didn't merge this commit: https://github.com/Anthony25/netbox-netprod-importer/pull/15/commits/6d9e4893ad5552a53439d572a31fc758d7f90be9

I don't really see what you're trying to do, neither why you duplicated the -p/--password option. Could you do a new PR specifically for this so we can discuss about it?

aruhier commented 5 years ago

By the way, for the new PR, you should go back to a clean master branch (branched from this repo) and cherry pick the commit from this PR that I didn't merge.

Thanks!