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

JSON Parser #38

Merged
merged 20 commits into from
Jun 18, 2017
Merged

JSON Parser #38

merged 20 commits into from
Jun 18, 2017

Conversation

bewing
Copy link
Member

@bewing bewing commented Apr 3, 2017

Basic JSON parser, first profile using it, and a bonus profile template for openconfig-platform.

This branch is based on #28

* develop:
  Fixed mappers to use real yang names
  Fixes \napalm-automation#36
  Don't assume device_config is a list of strings.
  Fix indent level in _exec_commands
  Added ansible examples to the tutorial
  Added ansible examples
  Bump version
  Updating requirements
  Load dict now update  even on defaults and added compliance_report to Root class

Conflicts:
	napalm_yang/parser.py
This parser can understand Python dictionaries from devices
and JSON strings injected into native.
component:
_parse:
mode: dict
from: "{%- set data = {} -%}{%- for k, v in bookmarks.components.0.0.items() -%}{%- if v is mapping and k is not equalto 'systemInformation' -%}{%- for kk, vv in v.items() -%}{%- do data.update({'{}_{}'.format(k,kk): vv}) -%} {%- endfor -%} {%- endif -%} {%- endfor -%} {{ data|json }}"
Copy link
Member Author

Choose a reason for hiding this comment

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

@dbarrosop commented in slack that this might be better implemented as a custom filter, which I had not thought of. Should we standardize device output transformation through filters?

Copy link
Member

Choose a reason for hiding this comment

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

If it can be generalized, yes.

Copy link
Member Author

@bewing bewing Apr 4, 2017

Choose a reason for hiding this comment

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

_parse_leaf_transform(cls, mapping):

Have a mapping["method"] to call? How would we make sure the method is imported? Looking at how complex transforming the json into the model is, doing the brunt of the transformation in python might be better.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we should have helpers in the driver classes to transform data for us?

@dbarrosop
Copy link
Member

I will pull your branch during the weekend and try to use it for parsing interfaces' state before merging.

* develop:
  Now you can have a closing steatement for list elements
  Unify naming
  Removed root BGP and added network instances with all protocols instead
  Update README.md
* init_native:
  Now you can have a closing steatement for list elements
  Unify naming
  Removed root BGP and added network instances with all protocols instead
  Update README.md
@dbarrosop dbarrosop self-assigned this Apr 16, 2017
@dbarrosop
Copy link
Member

Sorry for the late reply on this. This looks good but CI is broken :(

@bewing
Copy link
Member Author

bewing commented Jun 3, 2017 via email

Yes, the JSON in matches the JSON out.  Go figure.
@dbarrosop
Copy link
Member

I am holding onto this one for a few more days. I am going to implement some changes that might break your PR but don't worry about that. I will fix them myself and then merge :)

@dbarrosop
Copy link
Member

Can we decouple the JSONParser from the actual parsing of openconfig-platform? There are some things to discuss there so I wouldn't want to tie them. I'd also appreciate if you could parse the interfaces on EOS for example with this so we can add from the very begining some tests and documentation. If you are constrained with time let me know and we can probably merge this into a separate branch and collaborate.

@bewing
Copy link
Member Author

bewing commented Jun 11, 2017

Sure, will rebase today, and work on creating openconfig-interfaces eos profile

@dbarrosop
Copy link
Member

Great, thanks. Don't try to parse too much, something similar to the state parsing for junos interfaces is more than enough. I just want to have something to test on, specially as I might be refactoring some bits in the upcoming weeks. I am trying to make things a bit more efficient and simple to use.

* develop: (74 commits)
  add post-process-filter attribute to text parser (napalm-automation#59)
  Allow each parser to modify native on start (napalm-automation#28)
  Updated CHANGELOG
  Updated docs
  Removed negate_recursively as it's unnecessary
  Added network-instance/junos translator
  Added reuse and replace_on_merge
  When using lists of actions allow advancing containers in xml
  Added _translate_leaf_map
  Added config parser for network-instances/junos
  Added missing key
  Various improvement to xml parser
  Minor fix to network-instance/eos
  Added tests for network-instance/eos
  Added network-instance parser/translator for EOS
  We can now remove elements recursively in cases where the nesting of YANG model doesnt reflect the nesting in native configuration
  Minor fixes to the logic of the lists
  Propagate attribute name rather than yang_name
  Adapted network instances model to lists
  Find translation point per rule as before
  ...

Conflicts:
	napalm_yang/helpers.py
	napalm_yang/parser.py
@bewing
Copy link
Member Author

bewing commented Jun 17, 2017

Jesus that merge took longer to troubleshoot than I thought it would. Will work on replacing openconfig-platform with openconfig-interfaces for test case purposes next.

@bewing
Copy link
Member Author

bewing commented Jun 18, 2017

Couple of notes:

  • Couldn't figure out how to cross-reference in the parser -- IE, couldn't use parent_key from interfaces.interface as a dictionary key in bookmarks.interfaces.1, to collect ifIndex
  • Looks like the test interface doesn't really support multiple datasources? Might want to have a discussion regarding that.

@bewing
Copy link
Member Author

bewing commented Jun 18, 2017

Further note -- this profile is mostly just for testing, as it cannot correctly handle subinterfaces, which show up as regular interfaces in native. :/

@dbarrosop
Copy link
Member

It's alright. I am going to merge it as I want to re-do some parts of the whole parser framework and I will break stuff so having it merged will allow me to fix it myself rather than having to ask you again to do it yourself.

In case you are curious, I want to do three main changes to the framework:

  1. from will work like in in the translator. Which means that it will be only necessary to specify when you don't want to follow the "expected workflow" (if you capture an interface block the expectation will be that everything underneath will be working with that captured block so you will use from only if you want to use some other block instead)
  2. The bookmarks will work with references to objects rather than with strings, similar to how it works in the translator. I hope this will avoid a lot of serialization/deserialization and improve performance.
  3. mode will only be required when you don't want to use the "default" mode.

@dbarrosop dbarrosop merged commit 80cd38f into napalm-automation:develop Jun 18, 2017
@bewing bewing deleted the JSONParser branch June 18, 2017 15:41
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.

None yet

3 participants