Skip to content

feat: public api for embeds#6680

Merged
mindspank merged 43 commits intomainfrom
feat/emit-state-embed
Feb 21, 2025
Merged

feat: public api for embeds#6680
mindspank merged 43 commits intomainfrom
feat/emit-state-embed

Conversation

@mindspank
Copy link
Copy Markdown
Contributor

@mindspank mindspank commented Feb 14, 2025

Adds a RPC API for Rill embeds.

methods:

  • setState
  • getState

notifications:

  • stateChange

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
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

.replace("localhost:9091", "localhost:8081");
const accessToken = $page.url.searchParams.get("access_token");

onMount(() => {
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.

kept the initializing here if we want to support state for canvas in the future

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.

Makes sense. Though for now I'd propose this code would be easier-to-follow (more co-located) if this went in the initEmbedPublicAPI() function.

@mindspank
Copy link
Copy Markdown
Contributor Author

Closes #5490

Copy link
Copy Markdown
Contributor

@ericpgreen2 ericpgreen2 left a comment

Choose a reason for hiding this comment

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

Regarding the strict JSON-RPC 2.0 interface, I think it's too heavy to ask that host application developers implement their own JSON-RPC 2.0 over postMessage client (since there's no widely adopted NPM library for this). I looked around a little bit and didn't find any other BI tools that take this approach (correct me if I'm wrong).

Instead, I'd propose we either:

  1. Use a simple custom JSON interface, much like what Looker, Sigma, and Metabase offer.
  2. Build a Javascript SDK that hides the JSON-RPC 2.0 exchange under-the-hood.

I'd suggest we do Option 1 in the short-run and explore Option 2 in the long-run.

@mindspank mindspank force-pushed the feat/emit-state-embed branch from 355cd82 to 008d5b8 Compare February 17, 2025 21:59
.replace("localhost:9091", "localhost:8081");
const accessToken = $page.url.searchParams.get("access_token");

onMount(() => {
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.

Makes sense. Though for now I'd propose this code would be easier-to-follow (more co-located) if this went in the initEmbedPublicAPI() function.

Copy link
Copy Markdown
Contributor

@ericpgreen2 ericpgreen2 left a comment

Choose a reason for hiding this comment

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

Great feature. Big thanks – especially for the tests!

Comment on lines +69 to +75
await generateEmbed(
RILL_ORG_NAME,
RILL_PROJECT_NAME,
"bids_explore",
rillServiceToken,
);
const filePath = "file://" + path.resolve(__dirname, "..", "embed.html");
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.

Suggestion, so we don't have embed.html hardcoded in two places:

const embedPath = await generateEmbed(
  RILL_ORG_NAME,
  RILL_PROJECT_NAME,
  "bids_explore",
  rillServiceToken
);
const filePath = "file://" + path.resolve(embedPath);

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.

I'll keep the two versions for now since resolve is relative to the callers directory. Could make sense to move into a constant though since the file location would always have to be in the root of tests

@mindspank mindspank merged commit e6942b9 into main Feb 21, 2025
9 checks passed
@mindspank mindspank deleted the feat/emit-state-embed branch February 21, 2025 14:00
mindspank added a commit that referenced this pull request Feb 21, 2025
* feat: first rpc stub

* add emit

* feat: embed public api

* fix: better name for state stream

* fix: error handling and better spec conformatiy

* fix: cleanup

* fix: format

* fix: make id optional

* fix: first stub of tests

* Revert "fix: first stub of tests"

This reverts commit 2e33082.

* fix: clear method names

* fix: check input

* fix: gitignore

* fix: ignore

* Revert "fix: ignore"

This reverts commit 46ee2b3.

* Revert "fix: gitignore"

This reverts commit ec97c8e.

* fix: add tests

* Merge remote-tracking branch 'origin/main' into feat/emit-state-embed

* fix: comments

* fix: moving org and project to const

* fix: moving embed to test hook

* fix: format

* fix: read token

* fix: more fixes

* fix: comments

* fix: format

* .gitignore

* delete test file

* fix: pr comments

* fix: cleaner init

* fix: format

* fix: lint

* fix: more lint (in my pocket)

* fix: wait for bids

* fix: format

* fix: nits

* fix: format
grahamplata pushed a commit that referenced this pull request Feb 21, 2025
* feat: first rpc stub

* add emit

* feat: embed public api

* fix: better name for state stream

* fix: error handling and better spec conformatiy

* fix: cleanup

* fix: format

* fix: make id optional

* fix: first stub of tests

* Revert "fix: first stub of tests"

This reverts commit 2e33082.

* fix: clear method names

* fix: check input

* fix: gitignore

* fix: ignore

* Revert "fix: ignore"

This reverts commit 46ee2b3.

* Revert "fix: gitignore"

This reverts commit ec97c8e.

* fix: add tests

* Merge remote-tracking branch 'origin/main' into feat/emit-state-embed

* fix: comments

* fix: moving org and project to const

* fix: moving embed to test hook

* fix: format

* fix: read token

* fix: more fixes

* fix: comments

* fix: format

* .gitignore

* delete test file

* fix: pr comments

* fix: cleaner init

* fix: format

* fix: lint

* fix: more lint (in my pocket)

* fix: wait for bids

* fix: format

* fix: nits

* fix: format
briangregoryholmes added a commit that referenced this pull request Feb 26, 2025
…6763)

* fix(cloud-ui): redirect to login on expired session (#6733)

* Safer redirect (homepage)

* Safer redirect (CLI auth page)

* Safer redirect (GitHub connect pages)

* feat: public api for embeds (#6680)

* feat: first rpc stub

* add emit

* feat: embed public api

* fix: better name for state stream

* fix: error handling and better spec conformatiy

* fix: cleanup

* fix: format

* fix: make id optional

* fix: first stub of tests

* Revert "fix: first stub of tests"

This reverts commit 2e33082.

* fix: clear method names

* fix: check input

* fix: gitignore

* fix: ignore

* Revert "fix: ignore"

This reverts commit 46ee2b3.

* Revert "fix: gitignore"

This reverts commit ec97c8e.

* fix: add tests

* Merge remote-tracking branch 'origin/main' into feat/emit-state-embed

* fix: comments

* fix: moving org and project to const

* fix: moving embed to test hook

* fix: format

* fix: read token

* fix: more fixes

* fix: comments

* fix: format

* .gitignore

* delete test file

* fix: pr comments

* fix: cleaner init

* fix: format

* fix: lint

* fix: more lint (in my pocket)

* fix: wait for bids

* fix: format

* fix: nits

* fix: format

* Fix accidentally skipped migration 0057.sql (#6737)

* Canvas: add support for local time filters (#6673)

* support individual metrics view for time controls

* Add time controls for component

* Create new helper method for filters

* Working time filters

* Match dimension filter toggle to mocks

* Remove old filter references

* Lint fix

* Reset filters if metrics view is changed

* Use local comparison range label for KPI

* lint fix

* Enable grid

* Fix copy

* Disable comparison for local filters by default

* add image to concepts RD vs RC (#6729)

* fix: Support CSV and Table output for `rill query` --format (#6741)

* beautify csv and human output
* add comments

* Canvas: add stacked bar normalized (#6740)

* Canvas: tweak image component to fit all image sizes (#6705)

* Tweak: Do not show "New source" prompt if source has previously been ingested (#6616)

* Do not show "New source" prompt if the source has already once been ingested

* Add an "X" button in top-right of Alert Dialog

* feat: small improvements to search (#6736)

* Limit the max values in leaderboard to 15

* Add loading and error state for search

* Fix lint

* PR comments

* fix: global properties should be set with global scope (#6746)

* nit: trim semi colon in sql resolver (#6745)

* trim semi colon in sql resolver

* trim semi colon in sql resolver

* fix: check for homedir on rill start (#6744)

* fix: check for homedir on rill start

* fix: random test failure

* Revert "fix: random test failure"

This reverts commit a8a13ec.

* Use org name for autoscaler logs (#6747)

* Use org name for autoscaler logs

* Mark `dependency_error: true` for reconcile errors due to a dependency failure (#6732)

* mark  for reconcile errors due to a dependency failure

* review

* Update runtime/controller.go

Co-authored-by: Benjamin Egelund-Müller <b@egelund-muller.com>

* review comments

---------

Co-authored-by: Benjamin Egelund-Müller <b@egelund-muller.com>

* Add more context to billing and user logs (#6751)

* tweak: rewrite kpi component, canvas cleanup (#6730)

* wip

* cleanup and layout adjustments

* revert comment out

* revert change

* feedback

* feedback

* remove debug element

* force ci

* feat (Explore): Show the "last refreshed" date to everyone (#6750)

* Integrate with new `modelRefreshedOn` property

* Integrate with new `modelRefreshedOn` property (listings)

* Delete unused function

* Add comments

* Fix null date

* fix (microcopy): "source" -> "model" (#6754)

* Fix microcopy: "source" -> "model"

* Fix microcopy: "source" -> "model"

* fix: do not select if shift key is used (#6758)

* fix(explore): show error component not an infinite spinner (#6749)

* fix (exports): handle filters and searches (#6742)

* refactor: Push export logic into `ExportMenu` component

* Rename `CreateScheduledReportDialog` -> `ScheduledReportDialog`

* Handle scheduled reports

* fix: Account for both filters and searches

* Rename `scheduledReportsQueryArgs` -> `exportQueryArgs`

* Remove wrapper function

* Move fn to common location

* Make function easier-to-test

* Add unit tests

* Self review

* Fix time range for on-demand exports

* Fix lint

* Review

* Review pt 2

* fix: Use cached plan details for page load to avoid calling orb's APIs (#6739)

* Use cached plan details for page load

* Replicate plan type detection in frontend

* Fix lint

* Fix race condition in showing billing banner

* Fix all time in yaml crashing the app (#6760)

* Add organization billing plan name to default deployment annotations (#6761)

* init commit

* cleanup

* remove import

* fix (Cloud UI): Always redirect to login when auth token expires (#6766)

* Catch expired tokens in root `+layout.ts`

* Use native SvelteKit redirect (to avoid error page flash)

* e2e: Capture logs from Go services (#6771)

* Save `admin` and `runtime` output to `playwright/logs/admin-runtime.log`

* Upload logfile in GitHub Action

* fix panic due to uri parse error (#6772)

* remove log

* remove log

* add comment

* always show outline around selected components, hide comparison line in KPI

* reorg, cleanup and safari fixes

* type fix

---------

Co-authored-by: Eric P Green <ericpgreen2@gmail.com>
Co-authored-by: Alexander Thor <alec.karlsson@gmail.com>
Co-authored-by: Benjamin Egelund-Müller <b@egelund-muller.com>
Co-authored-by: Dhiraj Barnwal <sumankumaribarnwal@gmail.com>
Co-authored-by: Roy Endo <67675319+royendo@users.noreply.github.com>
Co-authored-by: Graham Plata <graham.plata@gmail.com>
Co-authored-by: Aditya Hegde <adityahegderocks@gmail.com>
Co-authored-by: Anshul Khandelwal <12948312+k-anshul@users.noreply.github.com>
Co-authored-by: Himadri Singh <himadri.singh@gmail.com>
Co-authored-by: Kasper Sjørslev <kaspersjo@users.noreply.github.com>
begelundmuller added a commit that referenced this pull request Feb 27, 2025
* Implement `rows:` in the canvas YAML

* Self review

* Fix test

* Potential fix for code scanning alert no. 31: Incorrect conversion between integer types

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

* Optional width/height

* Flatten inline component definitions

* Fix Go test

* feat: visual canvas editor yaml structure integration and refinement (#6763)

* fix(cloud-ui): redirect to login on expired session (#6733)

* Safer redirect (homepage)

* Safer redirect (CLI auth page)

* Safer redirect (GitHub connect pages)

* feat: public api for embeds (#6680)

* feat: first rpc stub

* add emit

* feat: embed public api

* fix: better name for state stream

* fix: error handling and better spec conformatiy

* fix: cleanup

* fix: format

* fix: make id optional

* fix: first stub of tests

* Revert "fix: first stub of tests"

This reverts commit 2e33082.

* fix: clear method names

* fix: check input

* fix: gitignore

* fix: ignore

* Revert "fix: ignore"

This reverts commit 46ee2b3.

* Revert "fix: gitignore"

This reverts commit ec97c8e.

* fix: add tests

* Merge remote-tracking branch 'origin/main' into feat/emit-state-embed

* fix: comments

* fix: moving org and project to const

* fix: moving embed to test hook

* fix: format

* fix: read token

* fix: more fixes

* fix: comments

* fix: format

* .gitignore

* delete test file

* fix: pr comments

* fix: cleaner init

* fix: format

* fix: lint

* fix: more lint (in my pocket)

* fix: wait for bids

* fix: format

* fix: nits

* fix: format

* Fix accidentally skipped migration 0057.sql (#6737)

* Canvas: add support for local time filters (#6673)

* support individual metrics view for time controls

* Add time controls for component

* Create new helper method for filters

* Working time filters

* Match dimension filter toggle to mocks

* Remove old filter references

* Lint fix

* Reset filters if metrics view is changed

* Use local comparison range label for KPI

* lint fix

* Enable grid

* Fix copy

* Disable comparison for local filters by default

* add image to concepts RD vs RC (#6729)

* fix: Support CSV and Table output for `rill query` --format (#6741)

* beautify csv and human output
* add comments

* Canvas: add stacked bar normalized (#6740)

* Canvas: tweak image component to fit all image sizes (#6705)

* Tweak: Do not show "New source" prompt if source has previously been ingested (#6616)

* Do not show "New source" prompt if the source has already once been ingested

* Add an "X" button in top-right of Alert Dialog

* feat: small improvements to search (#6736)

* Limit the max values in leaderboard to 15

* Add loading and error state for search

* Fix lint

* PR comments

* fix: global properties should be set with global scope (#6746)

* nit: trim semi colon in sql resolver (#6745)

* trim semi colon in sql resolver

* trim semi colon in sql resolver

* fix: check for homedir on rill start (#6744)

* fix: check for homedir on rill start

* fix: random test failure

* Revert "fix: random test failure"

This reverts commit a8a13ec.

* Use org name for autoscaler logs (#6747)

* Use org name for autoscaler logs

* Mark `dependency_error: true` for reconcile errors due to a dependency failure (#6732)

* mark  for reconcile errors due to a dependency failure

* review

* Update runtime/controller.go

Co-authored-by: Benjamin Egelund-Müller <b@egelund-muller.com>

* review comments

---------

Co-authored-by: Benjamin Egelund-Müller <b@egelund-muller.com>

* Add more context to billing and user logs (#6751)

* tweak: rewrite kpi component, canvas cleanup (#6730)

* wip

* cleanup and layout adjustments

* revert comment out

* revert change

* feedback

* feedback

* remove debug element

* force ci

* feat (Explore): Show the "last refreshed" date to everyone (#6750)

* Integrate with new `modelRefreshedOn` property

* Integrate with new `modelRefreshedOn` property (listings)

* Delete unused function

* Add comments

* Fix null date

* fix (microcopy): "source" -> "model" (#6754)

* Fix microcopy: "source" -> "model"

* Fix microcopy: "source" -> "model"

* fix: do not select if shift key is used (#6758)

* fix(explore): show error component not an infinite spinner (#6749)

* fix (exports): handle filters and searches (#6742)

* refactor: Push export logic into `ExportMenu` component

* Rename `CreateScheduledReportDialog` -> `ScheduledReportDialog`

* Handle scheduled reports

* fix: Account for both filters and searches

* Rename `scheduledReportsQueryArgs` -> `exportQueryArgs`

* Remove wrapper function

* Move fn to common location

* Make function easier-to-test

* Add unit tests

* Self review

* Fix time range for on-demand exports

* Fix lint

* Review

* Review pt 2

* fix: Use cached plan details for page load to avoid calling orb's APIs (#6739)

* Use cached plan details for page load

* Replicate plan type detection in frontend

* Fix lint

* Fix race condition in showing billing banner

* Fix all time in yaml crashing the app (#6760)

* Add organization billing plan name to default deployment annotations (#6761)

* init commit

* cleanup

* remove import

* fix (Cloud UI): Always redirect to login when auth token expires (#6766)

* Catch expired tokens in root `+layout.ts`

* Use native SvelteKit redirect (to avoid error page flash)

* e2e: Capture logs from Go services (#6771)

* Save `admin` and `runtime` output to `playwright/logs/admin-runtime.log`

* Upload logfile in GitHub Action

* fix panic due to uri parse error (#6772)

* remove log

* remove log

* add comment

* always show outline around selected components, hide comparison line in KPI

* reorg, cleanup and safari fixes

* type fix

---------

Co-authored-by: Brian Holmes <120223836+briangregoryholmes@users.noreply.github.com>
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