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 models for operator dashboard #74

Merged
merged 1 commit into from
Apr 29, 2024

Conversation

bmtcril
Copy link
Contributor

@bmtcril bmtcril commented Apr 22, 2024

Also removes a number of user_pii fields that are currently unused and cause ClickHouse to fail to build them when run at scale.

@Ian2012
Copy link
Contributor

Ian2012 commented Apr 22, 2024

@bmtcril how does this works on local and dev?

Where you able to query the data locally?

@bmtcril bmtcril force-pushed the bmtcril/operator_dash_enhancements branch 2 times, most recently from 36cbeea to 00762f3 Compare April 24, 2024 17:36
@bmtcril bmtcril requested review from Ian2012 and SoryRawyer April 24, 2024 17:54
config(
materialized="materialized_view",
schema=env_var("ASPECTS_XAPI_DATABASE", "xapi"),
engine=get_engine("SummingMergeTree()"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is our first use of a SummingMergeTree. I'm still trying to confirm this is all working as expected.

Copy link
Contributor

@SoryRawyer SoryRawyer Apr 25, 2024

Choose a reason for hiding this comment

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

Were you able to confirm that this does what we want? It looks correct to me but I'm trying to compare this to the AggregatingMergeTree table engine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's something off with it when there is new data coming in, I'm still debugging it

@bmtcril bmtcril force-pushed the bmtcril/operator_dash_enhancements branch 6 times, most recently from 5356cbc to fafa728 Compare April 26, 2024 11:55
Copy link
Contributor

@SoryRawyer SoryRawyer left a comment

Choose a reason for hiding this comment

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

I think this looks good! I tried testing the use of AggregatingMergeTree along with uniqCombinedState and it seemed to work as I'd expect.

@bmtcril bmtcril force-pushed the bmtcril/operator_dash_enhancements branch from fafa728 to 2fa70fd Compare April 29, 2024 13:41
@bmtcril bmtcril merged commit 4b78971 into main Apr 29, 2024
4 checks passed
@bmtcril bmtcril deleted the bmtcril/operator_dash_enhancements branch April 29, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants