ww-tech / lookml-tools

Tools to handle best practices for LookML dev. Contains three tools: LookML updater, linter, and grapher
Apache License 2.0
85 stars 16 forks source link

lookml-parser 5.0.0 breaks linter & grapher #2

Closed iacobus closed 5 years ago

iacobus commented 5 years ago

Hi. I believe this commit https://github.com/fabio-looker/node-lookml-parser/commit/ad755f2958c5717cb77971b3bcf3e262d0974382, included in version 5.0.0 of lookml-parser, breaks both the linter and the grapher (haven't tried the updater) due to simpler schema of the JSON representation of the LookML files. Apparently they removed the files array in favor of a dictionary, calling out the breaking change. With 5.0.0 you get the following error.

Traceback (most recent call last):
  File "run_grapher.py", line 42, in <module>
    main()
  File "run_grapher.py", line 39, in main
    LookMlGrapher(config).run()
  File "/Users/jacobo/workspace/lookml-tools/lkmltools/grapher/lookml_grapher.py", line 231, in run
    self.extract_graph_info(globstrings)
  File "/Users/jacobo/workspace/lookml-tools/lkmltools/grapher/lookml_grapher.py", line 220, in extract_graph_info
    self.process_file(filepath)
  File "/Users/jacobo/workspace/lookml-tools/lkmltools/grapher/lookml_grapher.py", line 190, in process_file
    if 'views' in json_data['files'][0]:
KeyError: 'files'

Reverting to lookml-parser version 4.0.0 resolves this issue.

weightwatchers-carlanderson commented 5 years ago

thanks @iacobus. I will look into it.

fabio-looker commented 5 years ago

Sorry guys, thought the semver change would be enough to prevent dependency issues. Let me know if there's something I could do to help here.

fabio-looker commented 5 years ago

In particular, it might make sense, for the file/files construct in particular, to expose an option to choose between object/array. (I would however argue in favor of only the object representation of other constructs within lookml itself)

weightwatchers-carlanderson commented 5 years ago

update: Josh Temple has just open sourced a pure Python LookML parser (https://github.com/joshtemple/lkml).

I am currently working to swap out the node parser with this Python one. I now have my test suite passing, so long as I don't have dupe keys in models (https://github.com/joshtemple/lkml/issues/6) or hanging commas (https://github.com/joshtemple/lkml/issues/5). If I/Josh can fix those issues in the python parser, then I will make a new lookml-tools release.

weightwatchers-carlanderson commented 5 years ago

@iacobus I've released v2.0.0. This uses a Python LookML parser so that there is no longer a dependency on this node parser.

iacobus commented 5 years ago

👍 will report if I encounter trouble. Thanks for the responsiveness @weightwatchers-carlanderson and @fabio-looker.