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

Support time spine configs for sub-daily granularity #10483

Merged
merged 17 commits into from
Jul 29, 2024
Merged

Conversation

courtneyholcomb
Copy link
Contributor

@courtneyholcomb courtneyholcomb commented Jul 24, 2024

resolves #10475

Description

Support configuration of multiple semantic layer time spines at different granularities. This will enable support for sub-daily granularity while maintaining query efficiency, since we can default to the most compatible time spine for a given query. YAML configs have been designed (by both SL and core PMs) to look like this:

models:
  - name: my_time_spine
    description: "my favorite time spine"
    time_spine:
      standard_granularity_column: date_day # column for the standard grain of your table
      custom_granularities_columns: # additional custom granularites for your timespine
        - name: fiscal_year
          offset: 2
        - name: my_other_column 
          offset: 3
    columns:
      - name: date_day
        granularity: day # set granularity at column-level for standard_granularity_column

Associated schema docs update: dbt-labs/schemas.getdbt.com#47
Note that the CI action Artifact Schema Check is failing because it is incompatible with the lint check. The linter requires empty lines at the bottom of the schema files, but the schema check requires no empty lines.

Checklist

  • I have read the contributing guide and understand what's expected of me.
  • I have run this code in development, and it appears to resolve the stated issue.
  • This PR includes tests, or tests are not required or relevant for this PR.
  • This PR has no interface changes (e.g., macros, CLI, logs, JSON artifacts, config files, adapter interface, etc.) or this PR has already received feedback and approval from Product or DX.
  • This PR includes type annotations for new and modified functions.

@cla-bot cla-bot bot added the cla:yes label Jul 24, 2024
@dbt-labs dbt-labs deleted a comment from github-actions bot Jul 24, 2024
@courtneyholcomb courtneyholcomb added user docs [docs.getdbt.com] Needs better documentation semantic Issues related to the semantic layer artifact_minor_upgrade To bypass the CI check by confirming that the change is not breaking labels Jul 24, 2024
@dbt-labs dbt-labs deleted a comment from codecov bot Jul 24, 2024
Copy link

codecov bot commented Jul 24, 2024

Codecov Report

Attention: Patch coverage is 97.22222% with 2 lines in your changes missing coverage. Please review.

Project coverage is 88.83%. Comparing base (cab6dab) to head (419aa3c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10483      +/-   ##
==========================================
- Coverage   88.86%   88.83%   -0.03%     
==========================================
  Files         180      180              
  Lines       22643    22704      +61     
==========================================
+ Hits        20121    20169      +48     
- Misses       2522     2535      +13     
Flag Coverage Δ
integration 86.06% <97.22%> (-0.15%) ⬇️
unit 62.24% <70.83%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
Unit Tests 62.24% <70.83%> (+0.01%) ⬆️
Integration Tests 86.06% <97.22%> (-0.15%) ⬇️

"Guidance on creating this model can be found on our docs site ("
"https://docs.getdbt.com/docs/build/metricflow-time-spine) "
"https://docs.getdbt.com/docs/build/metricflow-time-spine) " # TODO: update docs link!
Copy link
Contributor Author

@courtneyholcomb courtneyholcomb Jul 25, 2024

Choose a reason for hiding this comment

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

I have an SL team task up for this which is marked as blocked by the docs task.

@courtneyholcomb courtneyholcomb marked this pull request as ready for review July 25, 2024 01:16
@courtneyholcomb courtneyholcomb requested review from a team as code owners July 25, 2024 01:16
@courtneyholcomb courtneyholcomb requested review from jzhu13 and removed request for a team July 25, 2024 01:16
@github-actions github-actions bot added the community This PR is from a community member label Jul 25, 2024

.PHONY: json_schema
json_schema: ## Update generated JSON schema using code changes.
scripts/collect-artifact-schema.py --path schemas
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@MichelleArk MichelleArk merged commit 0a160fc into main Jul 29, 2024
63 of 66 checks passed
@MichelleArk MichelleArk deleted the court/time-spines2 branch July 29, 2024 17:39
@pestrov
Copy link

pestrov commented Aug 5, 2024

Hi @courtneyholcomb @MichelleArk

Sorry for commenting out of the left field, but I have a suspicion that this PR might have impacted our dbt versionless deployment and there is no guidance on how to fix our now broken project. The timing does seem to coincide and the support has no idea about this issue.

We have fivetran ad_reporting package that seems to have a semantic model and generates metricflow_time_spine inside.

Since August 1st we started getting error says about the deprecation of having this model without a YML reference, but links to a documentation page that has 0 references of how to actually mention this model.

Could it somehow be impacted by this change and can you suggest a remedy for us?
"dbt_version": "2024.8.221" fails and "2024.7.215" works fine.

Thanks!

The logs just say
Time spines without YAML configuration are in the process of deprecation. Please add YAML configuration for your 'metricflow_time_spine' model. See documentation to configure: https://docs.getdbt.com/docs/build/metricflow-time-spine

Object of type date is not JSON serializable
22:23:56 Traceback (most recent call last):
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/cli/requires.py", line 221, in wrapper
    result, success = func(*args, **kwargs)
                      ^^^^^^^^^^^^^^^^^^^^^
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/cli/requires.py", line 131, in wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/cli/requires.py", line 329, in wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/cli/requires.py", line 356, in wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/cli/requires.py", line 404, in wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/cli/requires.py", line 450, in wrapper
    ctx.obj["manifest"] = parse_manifest(
                          ^^^^^^^^^^^^^^^
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/parser/manifest.py", line 1971, in parse_manifest
    write_manifest(manifest, runtime_config.project_target_path)
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/parser/manifest.py", line 1948, in write_manifest
    manifest.write(path)
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/contracts/graph/manifest.py", line 1212, in write
    self.writable_manifest().write(path)
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/artifacts/schemas/manifest/v12/manifest.py", line 189, in write
    write_streaming_json(self, path)
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/artifacts/schemas/manifest/v12/write_streaming_json.py", line 20, in write_streaming_json
    _write_mapping_json("nodes", manifest.nodes, fp)
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/artifacts/schemas/manifest/v12/write_streaming_json.py", line 107, in _write_mapping_json
    _write_obj_json(val, fp)
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/artifacts/schemas/manifest/v12/write_streaming_json.py", line 116, in _write_obj_json
    json.dump(dct, fp)
  File "/usr/lib/python3.11/json/__init__.py", line 179, in dump
    for chunk in iterable:
  File "/usr/lib/python3.11/json/encoder.py", line 432, in _iterencode
    yield from _iterencode_dict(o, _current_indent_level)
  File "/usr/lib/python3.11/json/encoder.py", line 406, in _iterencode_dict
    yield from chunks
  File "/usr/lib/python3.11/json/encoder.py", line 406, in _iterencode_dict
    yield from chunks
  File "/usr/lib/python3.11/json/encoder.py", line 406, in _iterencode_dict
    yield from chunks
  File "/usr/lib/python3.11/json/encoder.py", line 439, in _iterencode
    o = _default(o)
        ^^^^^^^^^^^
  File "/usr/lib/python3.11/json/encoder.py", line 180, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type date is not JSON serializable```

@dbeatty10
Copy link
Contributor

@pestrov Adding the following within a file like models/_models.yml suppressed the "Time spines without YAML configuration" warning for me:

models:
  - name: metricflow_time_spine
    time_spine:
      standard_granularity_column: date_day
    columns:
      - name: date_day
        granularity: day

But I'm not sure what is causing the TypeError: Object of type date is not JSON serializable error you are getting.

If this issue persists for you, could you open an issue here with full details on how to replicate the issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
artifact_minor_upgrade To bypass the CI check by confirming that the change is not breaking cla:yes community This PR is from a community member semantic Issues related to the semantic layer user docs [docs.getdbt.com] Needs better documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Support for sub-daily granularity in semantic layer configs
4 participants