Added a new method for ISIS adjacency

[muffizone]

Pull request for get_isis_adjacencies method

napalm-automation/napalm-iosxr/pull/146

[coveralls]

Coverage increased (+0.08%) to 61.099% when pulling cf152c9 on muffadal-presswala:get_isis_neighbor into e21f10f on napalm-automation:develop.

[muffizone]

ietf_nsf_flag will change type to bool and not int

[coveralls]

Coverage increased (+0.08%) to 61.099% when pulling 9ad4a34 on muffadal-presswala:get_isis_neighbor into e21f10f on napalm-automation:develop.

[coveralls]

Coverage increased (+0.08%) to 61.099% when pulling 39b5a04 on muffadal-presswala:get_isis_neighbor into e21f10f on napalm-automation:develop.

[dbarrosop]

LGTM, @napalm-automation/council any comments?

[dbarrosop]

@muffadal-presswala I think you mentioned already you have some implementation ready as well, feel free to push it linking the PR to this one for reference. We usually like seeing a working implementation before merging new getters.

[dbarrosop]

Disregard my last commend, I just saw you already did, linking for reference: napalm-automation/napalm-iosxr/pull/146

[coveralls]

Coverage increased (+0.2%) to 61.183% when pulling b9ae673 on muffadal-presswala:get_isis_neighbor into e21f10f on napalm-automation:develop.

[muffizone]

Guys,

I was wondering if we need another method like get_isis_adjacencies_detail(). The plan was to add three additional keys in the neighbor attributes.

Same XML request is used to retrieve output for both of them.

Brief method output:

{system_id:
  {'interface_name': interface_name,
   'neighbor_state': neighbor_state,
   'circuit_type': circuit_type,
   'ietf_nsf_flag': ietf_nsf_flag,
   'network_type': network_type,
  }
}

Detailed method output

{system_id:
  {'interface_name': interface_name,
   'neighbor_state': neighbor_state,
   'circuit_type': circuit_type,
   'ietf_nsf_flag': ietf_nsf_flag,
   'network_type': network_type,
>    'neighbor_area': neighbor_area,
>    'neighbor_uptime': neighbor_uptime,
>    'topologies_supported': topologies_supported

  }
}

[dbarrosop]

Given the base information is the same I'd be inclined to only have the detailed version. Unless the detailed version has a huge cost in terms of querying the devices.

[muffizone]

Its the same query as base so should not make a difference

…

On Wed, Sep 20, 2017 at 9:23 AM, David Barroso ***@***.***> wrote: Given the base information is the same I'd be inclined to only have the detailed version. Unless the detailed version has a huge cost in terms of querying the devices. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#307 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIVHlQ4gYEhu-wyz-BtOH9Hmp1QQWxiuks5skMuUgaJpZM4PSCJ1> .

-- *Regards,* *Muffadal S Presswala*

[dbarrosop]

Then, unless someone has strong opinions about it I'd suggest sticking with the detailed one only.

[muffizone]

Ok will wait couple of days and push the changes soon for review

…

On Wed, Sep 20, 2017 at 11:30 AM, David Barroso ***@***.***> wrote: Then, unless someone has strong opinions about it I'd suggest sticking with the detailed one only. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#307 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIVHlcZuj5czAoZ0PdNI25xry9KGfTpuks5skOk6gaJpZM4PSCJ1> .

-- *Regards,* *Muffadal S Presswala*

[muffizone]

I have updated the method with additional output

[mirceaulinic]

Hi all - sorry for chiming in so late here. With the risk of being a PITA, wouldn't be better to return a dictionary with the same, but structured as per the openconfig-isis YANG model? This way we are sure we cover the fields defined there, we don't use a structure that we designed, and this could very well help with the napalm-yang too. Thoughts?

[muffizone]

Sure, sounds like a good idea.

Let me have a look and see what needs to change