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

Add new time spine configuration #314

Merged
merged 5 commits into from
Jul 16, 2024
Merged

Add new time spine configuration #314

merged 5 commits into from
Jul 16, 2024

Conversation

courtneyholcomb
Copy link
Contributor

@courtneyholcomb courtneyholcomb commented Jul 16, 2024

Resolves #280

Description

The time spine configuration has been redesigned to support new features (sub-daily granularity and custom calendar - designs here). This PR adds the new configuration needed for sub-daily granularity and marks the old one as in the process of deprecation. Best reviewed by commit to isolate the NodeRelation changes.

  • Deprecated config: Currently, the TimeSpineTableConfiguration is automatically set for all users in the core parser. This assumes the user has set up a model called metricflow_time_spine at DAY granularity, so that configuration is set automatically in the manifest. This config was never exposed to users in YAML. The only logic change made here for the legacy config is to make it no longer required in the JSON schema, which will help with eventual migration. The plan is to continue setting the old config automatically in the manifest until users have migrated over to the new one, using validation warnings to prompt them to migrate. MetricFlow will support both fields in the meantime.
  • NodeRelation changes: I renamed the NodeRelation implementation class PydanticNodeRelation to match all other implementation classes - previously the protocol and implementation classes had the same name. This will not be a breaking change for core/mantle because this class is never imported directly there. I also needed to move those classes to their own files to avoid circular imports. Viewing that commit separately should be helpful for review - the commit is purely copy/paste, no logic changes.
  • New config: I've included an example of what the YAML spec has been designed to look like below. Note that the top level key (semantic-layer) and the model key will be implemented in core instead of here.
semantic-layer:
  time_spines:
    - name: daily_time_spine
      model: ref('daily_time_spine')
      primary_column:
        name: created_at
        time_granularity: day

Checklist

@cla-bot cla-bot bot added the cla:yes label Jul 16, 2024
@courtneyholcomb courtneyholcomb changed the title Court/timespines Add new time spine configuration Jul 16, 2024
@dbt-labs dbt-labs deleted a comment from github-actions bot Jul 16, 2024
@courtneyholcomb courtneyholcomb marked this pull request as ready for review July 16, 2024 18:36
@courtneyholcomb courtneyholcomb merged commit 38773d3 into main Jul 16, 2024
58 checks passed
@courtneyholcomb courtneyholcomb deleted the court/timespines branch July 16, 2024 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Sub-daily granularity
2 participants