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

[MD] Support conditional rendering with global config #1978

Closed
Tracked by #1733
noCharger opened this issue Jul 27, 2022 · 8 comments
Closed
Tracked by #1733

[MD] Support conditional rendering with global config #1978

noCharger opened this issue Jul 27, 2022 · 8 comments
Assignees
Labels
multiple datasource multiple datasource project

Comments

@noCharger
Copy link
Contributor

No description provided.

@seraphjiang
Copy link
Member

@noCharger

what's the proposed config item name, please provide this, so other component owner could mock in their code before feature is implemented?

@noCharger
Copy link
Contributor Author

@noCharger

what's the proposed config item name, please provide this, so other component owner could mock in their code before feature is implemented?

data_source.enabled

@seraphjiang
Copy link
Member

we should have very simple switcher
e.g.

datasource.enabled: false

  1. this config should be readable from public plugin code at browser
  constructor(private readonly initializerContext: PluginInitializerContext) {
    this.config = this.initializerContext.config.get<ConfigType>();
  }
  1. this config should be accessible from Server plugin code

  2. this config could be configure and set when dashboards launch
    https://github.com/opensearch-project/OpenSearch-Dashboards/blob/2d6eef9679dcc225757873d50d40dccc8abeceea/test/common/config.js

  3. this config should be configure via yml config file

@seraphjiang
Copy link
Member

data_source.enabled

@zengyan-amazon @zhongnansu @kristenTian @mpabba3003 @yibow98 @KrooshalUX @kgcreative

here is the feature flag to enable/disable data source feature, would please review and comment if different opinion, otherwise we will go with it.

@noCharger
Copy link
Contributor Author

we should have very simple switcher

e.g.

datasource.enabled: false

  1. this config should be readable from public plugin code at browser

  constructor(private readonly initializerContext: PluginInitializerContext) {

    this.config = this.initializerContext.config.get<ConfigType>();

  }

  1. this config should be accessible from Server plugin code

  2. this config could be configure and set when dashboards launch

https://github.com/opensearch-project/OpenSearch-Dashboards/blob/2d6eef9679dcc225757873d50d40dccc8abeceea/test/common/config.js

  1. this config should be configure via yml config file

Aligned with all but the code block under 1 will not apply in our case since PluginConfigDescriptor interface and exposeToBrowser only supported for configs of same plugin

@seraphjiang
Copy link
Member

we should have very simple switcher
e.g.
datasource.enabled: false

  1. this config should be readable from public plugin code at browser

  constructor(private readonly initializerContext: PluginInitializerContext) {

    this.config = this.initializerContext.config.get<ConfigType>();

  }
  1. this config should be accessible from Server plugin code
  2. this config could be configure and set when dashboards launch

https://github.com/opensearch-project/OpenSearch-Dashboards/blob/2d6eef9679dcc225757873d50d40dccc8abeceea/test/common/config.js

  1. this config should be configure via yml config file

Aligned with all but the code block under 1 will not apply in our case since PluginConfigDescriptor interface and exposeToBrowser only supported for configs of same plugin

good to know. let's cut feature request issue to track for long term solution

to unblock this, what's proposal?

have we consider introduce plugin level flag?

data_source.cm.ui.enabled: false

@zhongnansu
Copy link
Member

zhongnansu commented Aug 16, 2022

There're 2 places we need conditional rendering.

  1. Cred management plugin and DS management plugin
  2. Index Pattern management plugin, to conditionally render the "attach datasource" configuration step

I agree to have only 1 switch in the data source plugin, and have Cred management plugin and DS management plugin to depend on that.

{
  "id": "dataSourceManagement",
  "version": "opensearchDashboards",
  "server": false,
  "ui": true,
  "requiredPlugins": ["management", "dataSource"],
  "optionalPlugins": [],
  "requiredBundles": ["opensearchDashboardsReact"]
}

As for how to access the global switch value. There can be 2 ways.

  1. A straightforward way is just to load the datasource plugin flag in yml, from Cred management plugin and DS management plugin setup stage. Haven't tried it yet, but I assume it should work
 this.initializerContext.config
      .create<DataSourceConfigType>()
      .pipe(first())
      .toPromise()
  1. Another way is to expose the flag from DataSourcePluginStart, as a function. Caller plugin can just access it during plugin setup stage. Similar to how indexManagementPlugin depends on data plugin
    export interface IndexPatternManagementStartDependencies {
    data: DataPublicPluginStart;
    }

cc @seraphjiang @zengyan-amazon @noCharger

@noCharger
Copy link
Contributor Author

There're 2 places we need conditional rendering.

  1. Cred management plugin and DS management plugin
  2. Index Pattern management plugin, to conditionally render the "attach datasource" configuration step

I agree to have only 1 switch in the data source plugin, and have Cred management plugin and DS management plugin to depend on that.

{
  "id": "dataSourceManagement",
  "version": "opensearchDashboards",
  "server": false,
  "ui": true,
  "requiredPlugins": ["management", "dataSource"],
  "optionalPlugins": [],
  "requiredBundles": ["opensearchDashboardsReact"]
}

As for how to access the global switch value. There can be 2 ways.

  1. A straightforward way is just to load the datasource plugin flag in yml, from Cred management plugin and DS management plugin setup stage. Haven't tried it yet, but I assume it should work
 this.initializerContext.config
      .create<DataSourceConfigType>()
      .pipe(first())
      .toPromise()
  1. Another way is to expose the flag from DataSourcePluginStart, as a function. Caller plugin can just access it during plugin setup stage. Similar to how indexManagementPlugin depends on data plugin
    export interface IndexPatternManagementStartDependencies {
    data: DataPublicPluginStart;
    }

cc @seraphjiang @zengyan-amazon @noCharger

Thanks Zhongnan, there are some difference in detail, feel free to check my PR :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
multiple datasource multiple datasource project
Projects
None yet
Development

No branches or pull requests

3 participants