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

Remove routing-instances from Junos "network-instance" model #141

Merged
merged 2 commits into from
Jun 20, 2018

Conversation

elkinaguas
Copy link
Contributor

As there is no routing instance in Junos, parse_config is not returning any data when using the network_instance model.

@coveralls
Copy link

coveralls commented Jun 15, 2018

Coverage Status

Coverage increased (+0.08%) to 84.109% when pulling 008bd81 on elkinaguas:develop into d98bee4 on napalm-automation:develop.

@cspeidel
Copy link
Contributor

@elkinaguas Junos does have routing instances, but in your case you were configuring BGP without a routing instance. So you need both lines:

 network-instances:
     _process:
-        - path: "configuration.routing-instances"
+        - path: "configuration"
           from: root_network-instances.0
     network-instance:
         _process:
             - path: "configuration"
               key: "{{ 'global' }}"
               from: root_network-instances.0
-            - path: "instance"
+            - path: "routing-instances.instance"

Also, the tests are failing because you need to add the appropriate changes to the unit tests. You can see them failing in the Travis ci build here: https://travis-ci.org/napalm-automation/napalm-yang/jobs/392896067

Running pytest in the napalm-yang directory before doing a pull request will help ensure a succesful PR.

@elkinaguas
Copy link
Contributor Author

Hi @cspeidel thanks for your comment, I'm running pytest with the PR I did and it shows me that all the tests are ok, do you know why?

~/yang$ pytest
============================= test session starts ==============================
platform linux -- Python 3.5.2, pytest-3.6.1, py-1.5.3, pluggy-0.6.0
rootdir: /home/elkin/yang, inifile:
collected 99 items                                                             

napalm-yang/test/integration/test_profiles.py .......................... [ 26%]
..............................                                           [ 56%]
napalm-yang/test/integration/test_tutorial.py .........                  [ 65%]
napalm-yang/test/integration/test_validate.py ..                         [ 67%]
napalm-yang/test/unit/test_jinja_filters.py .......                      [ 74%]
napalm-yang/test/unit/test_parser.py ...........                         [ 85%]
napalm-yang/test/unit/test_parser_resolve_path.py .......                [ 92%]
napalm-yang/test/unit/test_supported_models.py .....                     [ 97%]
napalm-yang/test/unit/test_text_tree.py .                                [ 98%]
napalm-yang/test/unit/test_to_dict.py .                                  [100%]

========================== 99 passed in 76.04 seconds ==========================

@dbarrosop
Copy link
Member

I just tested myself and I can confirm your patch is not 100% correct. Not sure why your tests are passing locally but I can reproduce the same erros by downloading your branch and testing locally. In any case, if you can fix your PR to include this change:

network-instances:
    _process:
        - path: "configuration"
          from: root_network-instances.0
    network-instance:
        _process:
            - key: "{{ 'global' }}"
            - path: "routing-instances.instance"
              key: name

then it should make the tests pass while fixing your problem :)

@dbarrosop
Copy link
Member

Thanks!

@dbarrosop dbarrosop merged commit 8f2bd90 into napalm-automation:develop Jun 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants