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

[SIP-145] Proposal for Embeddable Charts using the same mechanism as #17187 #30075

Closed
jayakrishnankk opened this issue Aug 30, 2024 · 8 comments
Labels
change:backend Requires changing the backend change:frontend Requires changing the frontend sip Superset Improvement Proposal

Comments

@jayakrishnankk
Copy link
Contributor

jayakrishnankk commented Aug 30, 2024

SIP-145 Proposal for Embeddable Charts using the same mechanism as #17187

Motivation

The existing method of embedding charts is using iframe method.
What this means is that the user have to be logged-in in parallel to the superset system for the chart to display without a login screen in the embedded mode.

Similar to embedding dashboards as outlined in SIP-75, superset dashboard developers would like to embed charts in a similar fashion so that the login page does not appear and a parallel login is not needed by the end user, avoiding the user to have an account created in superset to view the charts.

Proposed Change

This SIP is aimed to implement the Embedded Charts capability using the same GuestToken method that the Embeddable Dashboards are using.

New or Changed Public Interfaces

Added REST endpoints

GET    {host}/api/v1/chart/{pk}/embedded
POST   {host}/api/v1/chart/{pk}/embedded
DELETE {host}/api/v1/chart/{pk}/embedded

Added Views

In addition to existing view {host}/embedded/{uuid}, added the following

{host}/embedded/dashboard/{uuid} 
{host}/embedded/chart/{uuid}

NOTE: {host}/embedded/dashboard/{uuid} works the same way as {host}/embedded/{uuid}, which should be deprecated and removed in one of the subsequent versions.

New dependencies

None

Migration Plan and Compatibility

Needs a migration. Please see the attached merge request to see the migration details.
A new table called embedded_charts will be added and a foreign key reference will be made to the slices table.

Rejected Alternatives

Did not discuss alternate approaches

@jayakrishnankk jayakrishnankk added the sip Superset Improvement Proposal label Aug 30, 2024
@dosubot dosubot bot added change:backend Requires changing the backend change:frontend Requires changing the frontend labels Aug 30, 2024
@rusackas rusackas changed the title [SIP] Proposal for Embeddable Charts using the same mechanism as #17187 [SIP-144] Proposal for Embeddable Charts using the same mechanism as #17187 Sep 5, 2024
@Michila0
Copy link

Michila0 commented Sep 6, 2024

Is this done

@rusackas
Copy link
Member

rusackas commented Sep 6, 2024

Is this done

Not even close... the proposal has been numbered, but has not yet been put up for a discussion on the ASF list, which then would lead to a vote, which then would open the door for the work to be completed/merged.

@Michila0 do you have thoughts on the proposal you'd like to add to carry the discussion forward?

@Michila0
Copy link

Michila0 commented Sep 7, 2024

ASF list? I like contribute with you

@rusackas rusackas changed the title [SIP-144] Proposal for Embeddable Charts using the same mechanism as #17187 [SIP-145] Proposal for Embeddable Charts using the same mechanism as #17187 Sep 10, 2024
@rusackas
Copy link
Member

rusackas commented Sep 10, 2024

Discussion opened! Feedback welcome, and anyone volunteering to help contribute to this should chime in on the linked PR and/or reach out to @jayakrishnankk on slack or other means.

@rusackas
Copy link
Member

I'll open the vote on this, if everyone agrees it's ready.

@villebro
Copy link
Member

I'd like to add here that I think this is a natural progression of SIP-75, and could arguably have been considered being covered by that SIP already. So big thanks @jayakrishnankk for taking this on, this will be very valuable for cases where one wants to only embed a single chart, as opposed to a full dashboard.

@mistercrunch
Copy link
Member

Hey I just wanted to point out that an easy workaround is to create a single-chart dashboard and embed that, which may not fully cover all the use cases. It's not unlikely that in the future of Superset - once we allow for in-dashboard-chart-creation, that charts wouldn't exist outside the context of a dashboard.

@rusackas
Copy link
Member

rusackas commented Nov 6, 2024

Closing and moving this on the SIP board, since the VOTE thread has PASSED! Thanks @jayakrishnankk and thanks in advance for the implementation!

@rusackas rusackas closed this as completed Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:backend Requires changing the backend change:frontend Requires changing the frontend sip Superset Improvement Proposal
Projects
Status: [RESULT][VOTE] Approved
Development

No branches or pull requests

5 participants