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

Standardize manifest representation of YAML selectors #2879

Closed
jtcohen6 opened this issue Nov 10, 2020 · 2 comments · Fixed by #2895
Closed

Standardize manifest representation of YAML selectors #2879

jtcohen6 opened this issue Nov 10, 2020 · 2 comments · Fixed by #2895
Assignees
Labels
enhancement New feature or request

Comments

@jtcohen6
Copy link
Contributor

Describe the feature

We should standardize YAML selectors before writing them to manifest.json. This will make it easier for downstream consumers of the manifest to parse a single, standardized form.

YAML selectors are flexible: they can include fully qualified YAML dictionaries as well as CLI-style strings. These two selectors are functionally identical:

selectors:

  - name: shorthand
    description: "This uses more CLI-style syntax"
    definition:
      union:
        - intersection:
            - '@source:snowplow'
            - 'tag:nightly'
        - 'models/export'

  - name: longhand
    description: "This is a fuller YAML specification"
    definition:
      union:
        - intersection:
            - method: source
              value: snowplow
              childrens_parents: true
            - method: tag
              value: nightly
        - method: path
          value: models/export

Based on the work in #2866, dbt will write the 1:1 JSON dict representation to the selectors key in the manifest:

{
  "shorthand": {
    "name": "shorthand",
    "description": "This uses more CLI-style syntax",
    "definition": {
      "union": [
        {
          "intersection": [
            "@source:snowplow",
            "tag:nightly"
          ]
        },
        "models/export"
      ]
    }
  },
  "longhand": {
    "name": "longhand",
    "description": "This is a full YAML specification",
    "definition": {
      "union": [
        {
          "intersection": [
            {
              "method": "source",
              "value": "snowplow",
              "childrens_parents": true
            },
            {
              "method": "tag",
              "value": "nightly"
            }
          ]
        },
        {
          "method": "path",
          "value": "models/export"
        }
      ]
    }
  }
}

Instead, I think dbt should write the exact same representation to manifest.json, and that it should be the "longhand" version.

dbt already needs to do the work of parsing CLI-style strings and converting it to selector classes. The difficulty comes in digging up those converted representations. Our best bet here may be to extract functions that we can call in both places.

Describe alternatives you've considered

We could put the burden on artifact consumers to handle selectors whichever way they come.

@drewbanin Could you advise on how much of a lift that would be from a dbt-docs perspective? We already have to reimplement a lot of dbt selection logic in JS as it is...

Who will this benefit?

Users of YAML selectors (new in v0.18) who would want to access those selectors elsewhere, e.g. in dbt-docs DAG viz.

@drewbanin
Copy link
Contributor

@jtcohen6 i don't think that should be tremendously difficult - the dbt Docs site already knows how to parse apart and apply selector strings, so we just need to cobble together some of the existing graph selector methods to get this working :)

@jtcohen6
Copy link
Contributor Author

@drewbanin that's fair. We ended up standardizing :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants