Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with override_constant parameter in remote_dependency/local_dependency #75

Closed
kushal-i opened this issue Nov 14, 2022 · 2 comments
Labels
help wanted Extra attention is needed

Comments

@kushal-i
Copy link

kushal-i commented Nov 14, 2022

We have this situation in one of our project's manifest file:

project_name: "abc"

constant: app_name {
  value: "ABC"
}

constant: database_name {
  value: "ABC"
}

local_dependency: {
  project: "pqr"
  override_constant: app_name {
    value: "@{app_name}"
  }
  override_constant: database_name {
    value: "@{database_name}"
  }
}

remote_dependency: xyz {
  url: "git@github.com:xyz/xyz.git"
  ref: "main"
  override_constant: app_name {
    value: "@{app_name}"
  }
  override_constant: database_name {
    value: "@{database_name}"
  }
}

This looks to be a valid syntax in LookML, but when we try to parse it, we get this error:

KeyError: 'Key "override_constant" already exists in tree and would overwrite the existing value.'

The parser supports multiple constant keys at the top level, can it be updated to support multiple override_constant keys in the local_dependency and remote_dependency blocks?

@joshtemple
Copy link
Owner

Hey @kushal-i! lkml does not currently explicitly support parsing the project manifest file. As far as I can tell, constant isn't supported either, as parsing the following:

project_name: "abc"

constant: app_name {
  value: "ABC"
}

constant: database_name {
  value: "ABC"
}

Results in this output:

{
  "project_name": "abc",
  "constant": {
    "value": "ABC",
    "name": "database_name"
  }
}

which does not contain the first constant defined.

I would absolutely review a PR to support parsing the manifest file, but it's not currently a priority for me to implement. If you want to tackle it, you'd need to look in lkml/keys.py and make some additions to the PLURAL_KEYS tuple, which contains keys like view and dimension where multiple definitions should be allowed.

@joshtemple
Copy link
Owner

I'm going to close this issue in the current scope of the project, it is superseded by #77.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants