Skip to content

dashboards as mv #7645

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 40 commits into from
Aug 7, 2025
Merged

dashboards as mv #7645

merged 40 commits into from
Aug 7, 2025

Conversation

pjain1
Copy link
Member

@pjain1 pjain1 commented Jul 17, 2025 •
edited
Loading

Fixes https://linear.app/rilldata/issue/PLAT-10/dashboard-as-metrics-view-implementation

override - (add explore inline) - explore emitted by default, disable using no_explore: true

parent: ad_bids
display_name: Ad bids derived 

smallest_time_grain: "day"
timeseries: timestamp

parent_dimensions: 
  expr: "*"
...

explore:
...

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

@pjain1 pjain1 marked this pull request as draft July 17, 2025 06:11
@mindspank
Copy link
Contributor

Dont forget to update: https://github.com/rilldata/rill/blob/main/runtime/parser/schema/project.schema.yaml

@pjain1
Copy link
Member Author

pjain1 commented Jul 17, 2025

Dont forget to update: https://github.com/rilldata/rill/blob/main/runtime/parser/schema/project.schema.yaml

Sure will do after initial review

@pjain1 pjain1 marked this pull request as ready for review July 21, 2025 12:04
@pjain1 pjain1 requested a review from begelundmuller July 22, 2025 13:34
begelundmuller
begelundmuller requested changes Jul 23, 2025
View reviewed changes
@pjain1 pjain1 requested a review from begelundmuller July 25, 2025 18:07
begelundmuller
begelundmuller requested changes Jul 31, 2025
View reviewed changes
runtime/parser/parse_metrics_view.go
"fmt"
"strings"
"time"

runtimev1 "github.com/rilldata/rill/proto/gen/rill/runtime/v1"
"github.com/rilldata/rill/runtime/pkg/rilltime"
"golang.org/x/exp/maps"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can just import "maps"

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I saw that but it is same as in parse_explore and standard maps.Keys returns a iter.Seq[K] instead of []string, I think thats why it was used in other places in first place.

@begelundmuller
Copy link
Contributor

Also copying this request from @mindspank :

If we want to be able to non-breaking migrate old projects to this syntax, then I think supporting a custom name would be best:

type: metrics_view
...
explore:
  name: custom_name

@pjain1 pjain1 requested a review from begelundmuller August 1, 2025 14:32
begelundmuller
begelundmuller requested changes Aug 5, 2025
View reviewed changes
mindspank
mindspank reviewed Aug 6, 2025
View reviewed changes
@mindspank
Copy link
Contributor

Deferring review to @nishantmonu51 which is the PM for this effort

@mindspank mindspank requested review from nishantmonu51 and removed request for mindspank August 6, 2025 13:16
@pjain1 pjain1 merged commit af34efa into main Aug 7, 2025
14 checks passed
@pjain1 pjain1 deleted the dashboards_as_mv branch August 7, 2025 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mindspank mindspank mindspank left review comments

@begelundmuller begelundmuller begelundmuller approved these changes

@nishantmonu51 nishantmonu51 Awaiting requested review from nishantmonu51

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pjain1 @mindspank @begelundmuller @AdityaHegde