feat: Dynamically imported viz plugins #10288

suddjian · 2020-07-10T21:12:39Z

SUMMARY

Adds functionality to import plugins dynamically. A corresponding plugin can be found here: https://github.com/apache-superset/dynamic-import-demo-plugin

Goals:

Allow third parties to create plugins without changing Superset code
Allow users of Superset to add new chart types without backend access
Allow plugin authors to use whatever languages and code styles they prefer
Work towards a future improved contract between superset and third-party code.
Open the door to developing a future “app store” so that plugins can be conveniently searched and installed at the press of a button

How it works:

Plugin author starts with the template repo with a structure similar to existing plugins. Builds a bundle using Webpack.
Shared packages (react, lodash, @superset-ui) are provided superset-side and are referenced as externals from Webpack.
Bundle is hosted somewhere on the web - CDN, github etc. The plugin has serve command to help with local development.
Use the new Custom Plugin UI in Superset (under the "Manage" menu) to add the plugin to Superset.

Various hacks were necessary to build a PoC, but I believe they can all be solved with some kind of arcane webpack incantation:

Assigning modules to namespaced global objects to support a plugin's external dependencies instead of an actual dependency management system
Can’t async import from inside the plugin
Images/other assets. How do we load them if we don't know where the plugin is going to be hosted? Currently using url-loader with limit: Infinity, which works but is kinda sad.
I'm still tracking down all the places in Superset that need to change to respect async import of plugins.

I'd greatly appreciate any help addressing these issues, or any feedback on the code that anyone has!

TEST PLAN

toggle on the feature flag
run the example plugin linked above
add the example plugin from the "Plugins" settings nav menu item.

ADDITIONAL INFORMATION

amitmiran137 · 2020-08-15T05:07:17Z

@suddjian I'm happy to help out here

suddjian · 2020-08-16T16:19:14Z

Thanks for the offer @amitNielsen! I won't have a chance to get back on this until next week, but if we can address the CI errors I think it should be just about ready for review. Just needs one last pass to make sure that every part of the app knows how to wait for the async plugins.

If you make some fixes I can merge your branch 🙂

amitmiran137 · 2020-08-24T14:00:14Z

@suddjian I tried solving the issue in the explore page where controlsPanel section / row is not re-rendering the exiting parameters of the query like group by and metrics.

Can we maybe do some knowledge transfer on how what exactly should be done there and maybe do some refactoring in those areas

you can also have a look at my PR that solves few of the issues already
suddjian#2

codecov-commenter · 2020-08-28T00:14:39Z

Codecov Report

Merging #10288 into master will decrease coverage by 6.16%.
The diff coverage is 64.26%.

@@            Coverage Diff             @@
##           master   #10288      +/-   ##
==========================================
- Coverage   64.76%   58.59%   -6.17%     
==========================================
  Files         789      762      -27     
  Lines       37091    36142     -949     
  Branches     3555     3315     -240     
==========================================
- Hits        24021    21177    -2844     
- Misses      12963    14774    +1811     
- Partials      107      191      +84

Flag	Coverage Δ
#cypress	`55.52% <63.00%> (-0.10%)`	⬇️
#javascript	`?`
#python	`60.29% <71.11%> (+0.26%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset-frontend/src/chart/chartReducer.js	`62.50% <0.00%> (-6.36%)`	⬇️
superset-frontend/src/dashboard/App.jsx	`100.00% <ø> (ø)`
superset-frontend/src/explore/App.jsx	`100.00% <ø> (ø)`
...et-frontend/src/explore/reducers/exploreReducer.js	`42.62% <0.00%> (-4.84%)`	⬇️
superset-frontend/src/views/App.tsx	`100.00% <ø> (+42.10%)`	⬆️
...ns/versions/73fd22e742ab_add_dynamic_plugins_py.py	`0.00% <0.00%> (ø)`
superset/models/__init__.py	`100.00% <ø> (ø)`
superset/views/__init__.py	`100.00% <ø> (ø)`
superset/viz_sip38.py	`0.00% <0.00%> (ø)`
...omponents/DynamicPlugins/DynamicPluginProvider.tsx	`10.41% <10.41%> (ø)`
... and 300 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c715cad...2cf793f. Read the comment docs.

amitmiran137 · 2020-08-29T13:10:11Z

@suddjian I've fix the pre-commit issues for re configuring 3rd party and then re running sort
for some reason I can't pull request to your repo as last time 🤔

here is my branch with the fix : https://github.com/amitnielsen/incubator-superset/tree/dynamic-plugin-import

================ update =================
I've also removed a depracted test
added Loading component instead of '...loading'

amitmiran137 · 2020-08-30T08:42:09Z

@suddjian I suggest we also write an E2E cypress test for the explore page and dashboard page that loads a dynamic plugin stable

currently with the https://github.com/apache-superset/dynamic-import-demo-plugin
I can't say for certain that plugins work 100% , currently this plugin doesn't do the initial load currently

we should also stabalize the demo plugin to be aligned with the latest yeo generated plugin, which is what all community plugins are starting from

suddjian · 2020-08-31T21:43:21Z

A cypress test sounds like a good idea to me.

I have the example plugin working locally, maybe I can help there?

I think dynamic plugins in the long run will have a different set of requirements than static plugins. I'm thinking about adding some kind of manifest json file to specify the plugin name, key, thumbnail, version, and main module. That would allow the backend to inspect plugin info without loading the plugin in a js environment. We could build these things into the yeoman generator, but I think for now this should be considered a very experimental feature and subject to change.

ktmud

Thanks for the awesome work! I think exposing @superset-ui packages as globals is the right way to go. In the future, there will only be one package so it should become more or less more manageable. For security reasons, we want to limit what methods are exposed in the superset-ui package available to plugins, but that's another story.

superset-frontend/src/explore/App.jsx

ktmud · 2020-09-03T16:44:41Z

superset-frontend/src/components/DynamicPlugins/DynamicPluginProvider.tsx

+        '@superset-ui/style': import('@superset-ui/style'),
+        '@superset-ui/translation': import('@superset-ui/translation'),
+        '@superset-ui/validator': import('@superset-ui/validator'),
+      });


Should defineSharedModules be called outside of fetchAll or in a separate setup function? It would also be nice to make DynamicProviderProvider load only specific plugins on demand instead of all together.

I would like to do loading on demand, but the way I got the example plugin's webpack to work is it references global variables on the window. I intend to define a superset-plugin.json manifest file for dynamic plugins in the future, which would be a good place to put dependency info. For now I'd like to keep this minimal.

This design is also not ideal for third-party plugins that may want to add libraries for other plugins to depend on. Definitely should be improved.

superset-frontend/src/components/DynamicPlugins/DynamicPluginProvider.tsx

stale · 2020-11-26T22:17:21Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

lozbrown · 2020-12-01T12:28:43Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

Really bot, no! this would be super useful

suddjian · 2020-12-19T04:58:36Z

superset-frontend/src/explore/controlUtils.js

@@ -167,53 +170,59 @@ export function getControlState(controlKey, vizType, state, value) {
  );
 }

+const getMemoizedSectionsToRender = memoizeOne(


suddjian · 2020-12-19T05:05:48Z

superset-frontend/src/explore/store.js

-    }
-  });
+  Object.keys(controlsState)
+    .concat(Object.keys(inputFormData))


Dynamically imported plugins don't have any controls state available until after the plugin has loaded. This change makes it so that we use the fields from the chart's form data instead of only using the controls.

suddjian · 2020-12-19T05:07:50Z

superset-frontend/src/utils/getControlsForVizType.js

@@ -21,26 +21,29 @@ import memoize from 'lodash/memoize';
 import { getChartControlPanelRegistry } from '@superset-ui/core';
 import { controls } from '../explore/controls';

-const getControlsForVizType = memoize(vizType => {
+const memoizedControls = memoize((vizType, controlPanel) => {


had to change memoization again here

suddjian · 2020-12-19T05:09:15Z

superset/errors.py

@@ -15,7 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 # pylint: disable=too-few-public-methods,invalid-name
-from dataclasses import dataclass
+from dataclasses import dataclass  # pylint: disable=wrong-import-order


don't know why but every linter decided to lose its shit here.

What version of Python are you on? I'm guessing the ordering fails due to dataclasses being built-in on 3.7 but an addon in previous versions. But no need to fix here, I can clean this up later.

suddjian · 2020-12-19T05:12:26Z

superset-frontend/src/explore/components/ExploreViewContainer.jsx

@@ -122,138 +127,110 @@ const Styles = styled.div`
  }
 `;

-class ExploreViewContainer extends React.Component {


Converted this to a function component out of frustration with trying to change behavior. Didn't have the stamina to go all the way and convert it to typescript.

suddjian · 2020-12-19T05:29:15Z

superset/security/manager.py

+        "DatabaseAsync",
+        "DatabaseView",
+        "DruidClusterModelView",
+        "DynamicPlugin",


this makes it so that only admins can write to dynamic plugins endpoints

suddjian · 2020-12-19T06:24:52Z

superset-frontend/src/explore/components/ExploreViewContainer.jsx

-    if (title) {
-      document.title = title;
+  useEffect(() => {
+    if (wasDynamicPluginLoading && !isDynamicPluginLoading) {


this effect is the only functionality in this file that's actually new

pkdotson

LGTM! Super excited to see this within superset!

villebro

LGTM!

villebro · 2020-12-19T13:05:31Z

superset-frontend/spec/javascripts/explore/components/ExploreViewContainer_spec.jsx

+  // jest.spyOn(ReactAll, 'useContext').mockImplementation(() => {
+  //   return {
+  //     store,
+  //     subscription: new Subscription(store),
+  //   };
+  // });


nit: I prefer /* */ for multi-line comments

villebro · 2020-12-19T13:11:11Z

superset/errors.py

@@ -15,7 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 # pylint: disable=too-few-public-methods,invalid-name
-from dataclasses import dataclass
+from dataclasses import dataclass  # pylint: disable=wrong-import-order


What version of Python are you on? I'm guessing the ordering fails due to dataclasses being built-in on 3.7 but an addon in previous versions. But no need to fix here, I can clean this up later.

ktmud · 2020-12-21T04:49:09Z

superset-frontend/cypress-base/cypress/integration/chart_list/card_view.test.ts

@@ -107,11 +113,12 @@ describe('chart card view', () => {
    cy.get('[data-test="modal-cancel-button"]').click();
  });

-  it('should edit correctly', () => {
+  // flaky
+  xit('should edit correctly', () => {


Why did we nix so many tests? Were they causing problems in other PRs, too? If yes, I think it would be better if they were turned off in a separate PR so we can keep better track of them.

All of these tests failed and passed at various times, as well as on other PRs. We did a little investigation but haven't found the cause of flakiness yet.

betodealmeida · 2020-12-22T19:17:11Z

Migration is failing on MySQL with:

sqlalchemy.exc.OperationalError: (MySQLdb._exceptions.OperationalError) (1071, 'Specified key was too long; max key length is 3072 bytes')

suddjian · 2020-12-22T22:01:51Z

@betodealmeida That's very strange... Any idea why? It passed CI.

villebro · 2020-12-23T15:16:33Z

@betodealmeida what version of MySQL are you seeing this on?

dpgaspar · 2020-12-23T16:00:19Z

https://stackoverflow.com/questions/8746207/1071-specified-key-was-too-long-max-key-length-is-1000-bytes

Indicates that it could be related to the charset or a non InnoDB engine. Maybe?

superset-github-bot bot added the preset-io label Jul 10, 2020

pull-request-size bot added the size/XL label Jul 10, 2020

suddjian changed the title ~~Dynamically imported viz plugins~~ feat: Dynamically imported viz plugins Jul 11, 2020

suddjian force-pushed the dynamic-plugin-import branch from d5ebb30 to 42f97e7 Compare August 11, 2020 16:51

pull-request-size bot added size/XXL and removed size/XL labels Aug 14, 2020

ktmud reviewed Sep 3, 2020

View reviewed changes

stale bot added the inactive Inactive for >= 30 days label Nov 26, 2020

stale bot removed the inactive Inactive for >= 30 days label Dec 1, 2020

suddjian and others added 12 commits December 15, 2020 19:24

first attempts at dynamic plugin loading

fba6456

dynamic import working for explore

ad81f0b

memoize appropriately

8d7eaea

add a backend for dynamic plugins

9613e63

hack at getting dynamic plugins working with dashboards

6c15e2d

more work on making it work, + feature flag

d245784

lint

aad4757

actions to fix explore state when plugins load

c7061a5

handle dynamic control panel, functionify ExploreViewContainer

0333f26

fix: rearrange migrations branch

81e835c

fix: name and key as strings with length 50

eb44f49

bundle url length 2000

24e81a7

suddjian commented Dec 19, 2020

View reviewed changes

suddjian added 4 commits December 18, 2020 21:37

use makeApi

10f7a0d

flaky tests

0a8b10a

cleanup code

4f23ee6

flaaaakkkyyyyyy

2547086

suddjian commented Dec 19, 2020

View reviewed changes

suddjian force-pushed the dynamic-plugin-import branch from 521dccf to c0e0847 Compare December 19, 2020 06:38

dry

7e637ab

suddjian force-pushed the dynamic-plugin-import branch from c0e0847 to 7e637ab Compare December 19, 2020 06:39

Merge branch 'master' into dynamic-plugin-import

d0212d2

pkdotson approved these changes Dec 19, 2020

View reviewed changes

villebro approved these changes Dec 19, 2020

View reviewed changes

junlincc merged commit b5dd0f3 into apache:master Dec 19, 2020

ktmud reviewed Dec 21, 2020

View reviewed changes

betodealmeida added the risk:db-migration PRs that require a DB migration label Dec 22, 2020

ktmud mentioned this pull request Jan 20, 2021

[SIP-57] Proposal for Semantic Versioning #12566

Closed

junlincc mentioned this pull request Mar 30, 2021

[Chart] Create Andon chart apache-superset/superset-roadmap#159

Closed

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.0.0 labels Feb 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Dynamically imported viz plugins #10288

feat: Dynamically imported viz plugins #10288

suddjian commented Jul 10, 2020 •

edited by betodealmeida

Loading

amitmiran137 commented Aug 15, 2020

suddjian commented Aug 16, 2020

amitmiran137 commented Aug 24, 2020

codecov-commenter commented Aug 28, 2020 •

edited

Loading

amitmiran137 commented Aug 29, 2020 •

edited

Loading

amitmiran137 commented Aug 30, 2020

suddjian commented Aug 31, 2020 •

edited

Loading

ktmud left a comment •

edited

Loading

ktmud Sep 3, 2020

ktmud Sep 3, 2020

suddjian Dec 17, 2020

suddjian Dec 17, 2020

stale bot commented Nov 26, 2020

lozbrown commented Dec 1, 2020

suddjian Dec 19, 2020

suddjian Dec 19, 2020

suddjian Dec 19, 2020

suddjian Dec 19, 2020

villebro Dec 19, 2020

suddjian Dec 19, 2020

suddjian Dec 19, 2020 •

edited

Loading

suddjian Dec 19, 2020

suddjian Dec 19, 2020

pkdotson left a comment •

edited

Loading

villebro left a comment

villebro Dec 19, 2020

villebro Dec 19, 2020

ktmud Dec 21, 2020

suddjian Dec 22, 2020

betodealmeida commented Dec 22, 2020

suddjian commented Dec 22, 2020

villebro commented Dec 23, 2020

dpgaspar commented Dec 23, 2020

feat: Dynamically imported viz plugins #10288

feat: Dynamically imported viz plugins #10288

Conversation

suddjian commented Jul 10, 2020 • edited by betodealmeida Loading

SUMMARY

TEST PLAN

ADDITIONAL INFORMATION

amitmiran137 commented Aug 15, 2020

suddjian commented Aug 16, 2020

amitmiran137 commented Aug 24, 2020

codecov-commenter commented Aug 28, 2020 • edited Loading

Codecov Report

amitmiran137 commented Aug 29, 2020 • edited Loading

amitmiran137 commented Aug 30, 2020

suddjian commented Aug 31, 2020 • edited Loading

ktmud left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stale bot commented Nov 26, 2020

lozbrown commented Dec 1, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suddjian Dec 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pkdotson left a comment • edited Loading

Choose a reason for hiding this comment

villebro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

betodealmeida commented Dec 22, 2020

suddjian commented Dec 22, 2020

villebro commented Dec 23, 2020

dpgaspar commented Dec 23, 2020

suddjian commented Jul 10, 2020 •

edited by betodealmeida

Loading

codecov-commenter commented Aug 28, 2020 •

edited

Loading

amitmiran137 commented Aug 29, 2020 •

edited

Loading

suddjian commented Aug 31, 2020 •

edited

Loading

ktmud left a comment •

edited

Loading

suddjian Dec 19, 2020 •

edited

Loading

pkdotson left a comment •

edited

Loading