Skip to content

The final sync of DSI to MetricFlow#2001

Open
QMalcolm wants to merge 2 commits intomainfrom
qmalcolm--sync-dsi-final
Open

The final sync of DSI to MetricFlow#2001
QMalcolm wants to merge 2 commits intomainfrom
qmalcolm--sync-dsi-final

Conversation

@QMalcolm
Copy link
Copy Markdown
Contributor

We're collapsing DSI into MetricFlow. This is the final sync to do that, and in the coming weeks we'll archive the DSI repository.

@QMalcolm QMalcolm requested a review from a team as a code owner March 25, 2026 23:15
@cla-bot cla-bot bot added the cla:yes label Mar 25, 2026
@QMalcolm QMalcolm force-pushed the qmalcolm--sync-dsi-final branch from 956e179 to dc708b4 Compare March 25, 2026 23:17
@dbt-labs dbt-labs deleted a comment from github-actions bot Mar 26, 2026
@QMalcolm
Copy link
Copy Markdown
Contributor Author

QMalcolm commented Mar 26, 2026

This is a stacked PR

Copy link
Copy Markdown
Contributor

@theyostalservice theyostalservice left a comment

Choose a reason for hiding this comment

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

Approved, but with the caveat that I'm concerned about the type ignores, and if we can avoid them, I'd really prefer that.

"join_to_timespine": {"type": "boolean"},
"fill_nulls_with": {"type": "integer"},
"metric_aggregation_params": {"$ref": "metric_aggregation_params_schema"},
"is_private": {"type": "boolean"},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

when was the last time this was synced? this change is really old

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The last time we synced I believe was in October of 2025 via #1895

)

def __init_subclass__(cls, **kwargs) -> None:
def __init_subclass__(cls, **kwargs: Any) -> None: # type: ignore[misc]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same concern

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmmmmm I'll see what I can do about **kwargs typing. There is Unpack but I've never used it before 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The most common thing we do in the repository is that when **kwargs is present is that we don't type it, and we set # type: ignore[no-untyped-def]. I am going to go down that path. There are some places when we know exactly the shape we expect of **kwargs, and thus we type it. But in this case we're defining it on an ABC generic inheritable class, and we don't know the expected shape of **kwargs in the least 🫠

time: 2026-03-25T18:16:21.259009-05:00
custom:
Author: QMalcolm
Issue: NA
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please replace this with 2001 :P

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Core habits die hard 🙈

@QMalcolm QMalcolm force-pushed the qmalcolm--sync-dsi-final branch from dc708b4 to 53ee89b Compare March 28, 2026 00:16
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.

2 participants