Skip to content
This repository has been archived by the owner on Sep 17, 2019. It is now read-only.

issue 116 - implemented request to get routes for ipv6 destinations #117

Merged
merged 9 commits into from
Apr 28, 2017
Merged

issue 116 - implemented request to get routes for ipv6 destinations #117

merged 9 commits into from
Apr 28, 2017

Conversation

manuel-domke
Copy link
Contributor

@manuel-domke manuel-domke commented Apr 4, 2017 •
edited
Loading

implemented the xml request to get static routes for ipv6 destinations from #116

@coveralls
Copy link

coveralls commented Apr 4, 2017 •
edited
Loading

Coverage Status

Coverage decreased (-0.3%) to 87.302% when pulling ecb6eb1 on DENICeG:develop into 6ebe2cb on napalm-automation:develop.

@coveralls
Copy link

coveralls commented Apr 4, 2017 •
edited
Loading

Coverage Status

Coverage decreased (-0.3%) to 87.302% when pulling 205bcc2 on DENICeG:develop into 6ebe2cb on napalm-automation:develop.

@manuel-domke
Copy link
Contributor Author

manuel-domke commented Apr 4, 2017 •
edited
Loading

It seems to be impossible to improve code coverage (within scope of this project).

The test case for napalm_base only tests IPv4 at this time:
See https://github.com/napalm-automation/napalm-base/blob/develop/napalm_base/test/getters.py line 354ff:
@wrap_test_cases
def test_get_route_to(self, test_case):
"""Test get_route_to."""
destination = '1.0.4.0/24'

Test added in napalm-automation/napalm-base#233

@dbarrosop dbarrosop requested a review from mirceaulinic April 6, 2017 11:43
mirceaulinic
mirceaulinic suggested changes Apr 20, 2017
View reviewed changes
Copy link
Member

@mirceaulinic mirceaulinic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small change please and we're ready to get this in.
Thanks @manuel-domke

napalm_iosxr/iosxr.py Outdated
try:
ipv = IPAddress(network).version
except AddrFormatError:
return {'error': 'Wrong destination IP Address!'}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please raise TypeError instead or returning a dictionary.

@mirceaulinic mirceaulinic added this to the 0.5.0 milestone Apr 20, 2017
@coveralls
Copy link

coveralls commented Apr 28, 2017 •
edited
Loading

Coverage Status

Coverage decreased (-0.3%) to 87.302% when pulling 6941cba on DENICeG:develop into 6ebe2cb on napalm-automation:develop.

@mirceaulinic mirceaulinic mentioned this pull request Apr 28, 2017
Closed
@mirceaulinic mirceaulinic merged commit eb9366b into napalm-automation:develop Apr 28, 2017
@mirceaulinic
Copy link
Member

Thanks @manuel-domke @thorko @juergen-geinitz

@mirceaulinic
Copy link
Member

This will be included in the next version, 0.5.0. Hope to be able to release it this week.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mirceaulinic mirceaulinic mirceaulinic approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
0.5.0
Development

Successfully merging this pull request may close these issues.
5 participants
@manuel-domke @coveralls @mirceaulinic @thorko @juergen-geinitz