MohamedHalat commented on PR #31331: URL: https://github.com/apache/superset/pull/31331#issuecomment-2528989298
> This seems like a reasonable feature, but leaves me with a couple of questions: > > 1. I defer to @yousoph / @eschutho to assign a reviewer who might have a good embedded test setup. We need to figure out who are the best owners/reviewers for Embedded SDK changes, going forward. > 2. Regarding the feature flag, do you think we need to have a flag at all? Do you perceive a risk (security or otherwise) that would warrant people disabling it? > 3. If it's to be configurable, should the config happen at the Superset level, or can it be contained as part of the SDK itself? Hey @rusackas, thanks for the initial followup. To answer your questions: 2. I'm hesitant to remove the feature flag as I'd rather not add a `store.subscribe` that does nothing. 3. I'm all for having configuration be apart of the SDK, as long as the data passed is not too large. For now my only requirement is to have data masks emitted - which is why I renamed the feature flag. I can imagine it may be useful to have more information about the charts passed to the window parent. I'll come up with a small list of data to emit and If we are in agreement I'll add that data as well (possibly configured through SDK), for now I feel this amount is enough. -- 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: notifications-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
