villebro commented on issue #44989:
URL: https://github.com/apache/superset/issues/44989#issuecomment-6004350179
Thanks @msyavuz! I've updated the SIP-231 body to align with this proposal
and the feedback on #44875. A few points from the SIP-231 side:
- **Naming:** persisted instances live in a `widgets` table (model `Widget`,
the same way `slices` holds `Slice` rows), with
`widget_editors`/`widget_viewers`, and the `widget_type` column references the
registered widget class. REST is `/api/v1/widget/{uuid}` (plus `/data`), and
widget type metadata lives under `/api/v1/widget_type/{widget_type}/…`. Could
you update the `WidgetInstance` table and `widget_id` references here? For
consistency, the inline placement key could be `widgetType` too.
- **`WidgetBehavior` and `WidgetUi`:** SIP-231 notes that this SIP adds the
container, layout and filter-scope fields, so the list in §4 can stay here.
- **Drafts are optional (open questions 2 and 3):** a complete, valid
instance can be written directly, so `add`/`set_props`/`patch_props` with full
validation work as proposed. Drafts cover the two gaps: an unfinished instance
is built up in a draft rather than on the canvas until it validates, and the
draft preview data route shows its data before anyone else sees it. Committing
a draft of an inline instance is just a `set_props`. Whether edits should be
grouped by an explicit commit or persisted live with version history is still
open, so SIP-231 lists it as an open question.
- **Runtime choices (`x-runtime`, `runtime_props`):** I'd like M1 to stick
to additive runtime filters. Customizations let viewers reshape queries, so
they need safeguards first, such as a permission to change runtime choices.
Could this move from "Changes this SIP asks of SIP-231" to a later phase?
SIP-231 lists it as an open question for now.
- **Flag:** agreed on one shared `CANVAS` flag that turns on by default only
when both reach parity. One implementation detail: the widget REST API is
registered unconditionally and answers 404 while the flag is off, so its FAB
permissions exist and roles can be prepared before the flag is turned on, while
the MCP tools are removed. Could the canvas API follow the same pattern?
- **Event bus:** following Michael's feedback on #44875, the typed hooks
(`useRuntimeFilters`, `emitFilter`) sit on a generic bus that accepts any event
type, so extension widgets can emit their own events. It would be great if the
canvas bus kept the event kind open-ended too.
- **Async and realtime:** SIP-231 data serving is now async-first through
the Global Task Framework with realtime delivery (SIP-227), in line with my
earlier comment about replacing the 5-second polling.
I'll open a shared feature branch with an umbrella draft PR shortly, so we
can both start PRing into it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
