-
Notifications
You must be signed in to change notification settings - Fork 96
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
Update MF to support breaking changes in DSI for custom calendar #1522
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for handling so quickly!!
@@ -151,5 +155,7 @@ def create(self, name: str, entity_path: Sequence[str] = ()) -> WhereFilterDimen | |||
rendered_spec_tracker=self._rendered_spec_tracker, | |||
element_name=structured_name.element_name, | |||
entity_links=tuple(EntityReference(entity_link_name.lower()) for entity_link_name in entity_path) | |||
+ structured_name.entity_links, | |||
+ tuple( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change is just to support case insensitivity right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope, we switched from using DunderedNameFormatter
to StructuredLinkableSpecName
and that didn't have entity_links: Tuple[EntityReference, ...]
, but only entity_link_names: Tuple[str, ...]
. Now that I think about it, i'll just add a property to make entity_links
work in StructuredLinkableSpecName
so that we don't need this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooo ok sounds good!
Context
We merged 2 breaking changes in DSI dbt-labs/dbt-semantic-interfaces#363 and dbt-labs/dbt-semantic-interfaces#365 which changed most spec typing that used time granularity to be a
str
instead ofTimeGranularity
to enable support for custom granularity. Similarly, there were additional breaking changes to the objects that requires passing incustom_granularity_names
. This PR updates all those callsites to be compatible with the new version of DSI (to be released)Resolves SL-3097