lkml icon indicating copy to clipboard operation
lkml copied to clipboard

lkml.dump deletes "name" key from dict

Open victoryifei opened this issue 3 years ago • 4 comments

dump function mutates input dict by removing all "name" fields in the dict. Looks like it is caused by a .pop call on this line.

victoryifei avatar Aug 16 '21 23:08 victoryifei

Please provide an example and explain why this behavior doesn't work for your use case. Thanks!

joshtemple avatar Aug 17 '21 00:08 joshtemple

I was surprised that our test was broken. I used a constant variable for a dimension_group and I reused this same constant variable in multiple tests. The second test broke because in the first test the name in that constant variable was removed by lkml and the output lookml did not have a name for the dimension_group.

In general, mutating input variable can cause unexpected behavior unless the function is made to do that.

victoryifei avatar Aug 27 '21 22:08 victoryifei

@joshtemple This is also breaking for us for the same reason as @victoryifei described. Here is a minimum working example using lkml=1.2.0:

In [1]: import lkml
   ...:
   ...: MY_JOINS_CONSTANT = [{"name": "foreign_key_table", "sql_on": "sql_goes_here"}]
   ...:
   ...:
   ...: print(f"{MY_JOINS_CONSTANT=}")
   ...:
   ...: explores = [{"joins": MY_JOINS_CONSTANT}]
   ...: model_lkml = {"datagroups": [{"explores": explores}]}
   ...: lkml.dump(model_lkml)
   ...:
   ...: print(f"{MY_JOINS_CONSTANT=}")
   ...:
MY_JOINS_CONSTANT=[{'name': 'foreign_key_table', 'sql_on': 'sql_goes_here'}]
MY_JOINS_CONSTANT=[{'sql_on': 'sql_goes_here'}]

We are using that constant in more than one place and were not expecting lkml.dump() to modify it. The above comment describes both our usecase and the problem.

Let us know if there's any more detail that can be provided to help make the difficulty clear/what we can do to offer a fix for the issues, and thanks so much for providing and maintaining this absolutely excellent library!

ryan-duve avatar May 23 '22 13:05 ryan-duve

Thanks @ryan-duve for the example! I understand this issue better now and I'll get a fix pushed out in the next few days where we don't mutate that input directly.

joshtemple avatar May 23 '22 14:05 joshtemple

@joshtemple I have another case where name is being removed, but slightly different from the thread so far:

Minimal example:

  • albums.view.lkml
view: albums {
  sql_table_name: Album ;;


  dimension: id {
    primary_key: yes
    type: number
    sql: ${TABLE}.AlbumId ;;
  }
}
import lkml

with open('01_order_items.view.lkml', 'r') as file:
  parsed = lkml.load(file)

print(lkml.dump(parsed))
parsed # This line seems to be where the `name`s are removed
print(lkml.dump(parsed))

The output looks like this:

view: order_items {
  sql_table_name: Album ;;

  dimension: id {
    primary_key: yes
    type: number
    sql: ${TABLE}.AlbumId ;;
  }
}
view: {
  sql_table_name: Album ;;

  dimension: {
    primary_key: yes
    type: number
    sql: ${TABLE}.AlbumId ;;
  }
}

As you can see, the first time it is dumped and printed the names are present both for the view and the dimension, but the second time around both are missing.

My use-case is to try to generate the SQL (and execute it) for a given block of LookML and appropriate user-input, rather than to make any edits to the LookML itself.

samjewell avatar Oct 24 '22 10:10 samjewell

Thanks for your patience on this one. I've released version 1.3.1 which has this change. Please give it a try and let me know if it fixes your issue.

joshtemple avatar Oct 25 '22 17:10 joshtemple