Skip to content

Comments

feat: setup configuration-ui plugin registry#2886

Merged
likyh merged 53 commits intoapache:mainfrom
merico-ai:jc-2882-plugin-registry
Oct 31, 2022
Merged

feat: setup configuration-ui plugin registry#2886
likyh merged 53 commits intoapache:mainfrom
merico-ai:jc-2882-plugin-registry

Conversation

@e2corporation
Copy link
Contributor

@e2corporation e2corporation commented Aug 30, 2022

🆘 Config-UI / Plugin Registry (Phase I)

  • Create a Defined Plugin Registration Schema (JSON Registration Files)
  • Create Local Plugin Registry Configuration
    • JIRA
    • GitLab
    • GitHub
    • Jenkins
    • TAPD
    • AE
    • DBT
    • StarRocks
    • RefDiff
    • GitExtractor
    • Feishu
    • Azure 🆕
    • Bitbucket 🆕
    • Gitee 🆕
  • Implement Integrations Manager Hook @hooks/useIntegrations.jsx
    • Create Plugin and other related Data Models
    • Define Integrations Data Configuration Objects
    • Validate JSON Plugin Configuration
  • Create Integrations Context & Provider (@store/integrations-context.jsx)
  • Create Basic Documentation for Plugin JSON Registration
  • Refactor Integrations Data Dependencies on @data/Providers.js & @data/integrations.jsx
  • Refactor Transformations Manager Hook (Setup Default Transformations in Plugin Registry)
  • Fix Build Menu Configuration Dynamically
  • Fix Build All Connection Sources Dynamically
  • Fix Build Data Sources Connection List Dynamically
  • Feat Use Higher Order Component (HOC) Pattern for Transformation Settings Load
  • Chore Update Config-UI README.md
  • Chore Code Cleanups & Linting
  • TEST Data Connections Workflow
  • TEST Blueprints Workflow

Description

This PR creates a Plugin Registry on Configuration-UI as outlined on #2882. The new Integrations Manager Hook now provides a Dynamic API Configuration for Data Providers that replaces the static configuration formerly defined in the @data/Provider.js and @data/integrations.jsx global data constants. An Integrations Context has also been created to offer access to Dynamic Provider configuration to deeply nested child components and other Hook Services, when direct access to the IM Hook is unavailable or otherwise not desired.

As the Configuration API is preserved, Developers can continue to reference Provider Configuration objects in the same fashion as before without having to worry about maintaining multiple static configuration objects in multiple files.

Developer Notes

For developers, access to the Dynamic Plugin Registry API Configuration can be done in the following ways, by using the Integrations Hook or the Integrations Context. With the Context, usage can be done in 2 ways via the React.useContext Hook or with using a Context Consumer & React Render Function.

1. Integrations Hook @hooks/useIntegrations.jsx
For Top-Level Page & Service Components, import and initialize the Integrations Manager Hook. Simply de-structure the specific configuration objects and properties needed.

// Sample Hook Implementation (Abstract)
import React from 'react'
import useIntegrations from '@/hooks/useIntegrations'

class MyPageComponent {

 // Initialize Integrations Hook
 const {
    registry,
    plugins: Plugins,
    integrations: Integrations,
    activeProvider: IntegrationActiveProvider,
    Providers, 
    ProviderFormLabels,
    ProviderFormPlaceholders,
    ProviderConnectionLimits,
    setActiveProvider: setIntegrationActiveProvider
  } = useIntegrations()

 return (
   <>
       {Integrations.map((integration) => (<div>{integration.name}</div>) }
   </>
 )

}

2. Integrations Context + React.useContext Hook @store/integrations-context.jsx
Since Hooks can only be accessed by Top-Level Components, secondary Child and deeper nested components will need to use the Context for accessing provider configuration. Use object de-structure to reference the Configuration Objects needed.

// Sample Context + useContext Implementation (Abstract)
import React, { useContext } from 'react'
import IntegrationsContext from '@/store/integrations-context'

const MyBlueprintComponent = (props) => {

 // Initialize Integrations Context
  const { 
    Integrations,
    Providers,
    ProviderIcons,  
    ProviderLabels 
  } = useContext(IntegrationsContext)

  return (
    <ul>
      <li>{ProviderLabels[Providers.JIRA]}</li>
      <li>{ProviderLabels[Providers.GITHUB]}</li>
      <li>{ProviderLabels[Providers.JENKINS]}</li>
    </ul>
  )
    
}

3. Integrations Context + Consumer Render Function @store/integrations-context.jsx
In cases where we don't need access to configuration at the Class or Function level, but only in specific areas of the Render, the Consumer Pattern with a React render function can also be used. Use object de-structure to reference the Configuration Objects needed.

// Sample Context + Consumer Implementation (Abstract)
import React from 'react'
import IntegrationsContext from '@/store/integrations-context'

const MyNestedChildBlueprintComponent = (props) => {

  // Consumer + Render Function
  return (
        <IntegrationsContext.Consumer>
           {({ Providers, ProviderIcons }) => (
              <span style={{ marginLeft: '20px' }}>
                <span
                  className='iProvider-icon'
                >
                  {ProviderIcons[Providers.GITHUB] ? (
                    ProviderIcons[Providers.GITHUB](14, 14)
                  ) : (
                    <></>
                  )}
                </span>
              </span>
            )}
       </IntegrationsContext.Consumer> 
 )
}

The Active Provider (activeProvider)
The Active Data Provider is now managed by the Integrations Manager Hook (@/hooks/useIntegrations.jsx). After Plugin Registration, the activeProvider state variable will be an Instance of Plugin Data Model (@models/Plugin.js). The Provider Configuration is also accessible via the active provider when needed.

// Using Active Provider & useIntegrations Hook (Abstract)
import React, { useEffect } from 'react'
import useIntegrations from '@/hooks/useIntegrations'

const MyDevLakeComponent = (props) => {

  // Initialize Integrations Hook
  const {
    activeProvider,
    setActiveProvider
  } = useIntegrations()

  useEffect(() => {
    // Active Provider (Plugin Instance)
    console.log('>>> MY ACTIVE DATA PROVIDER = ', activeProvider)
      
     // Data Entities?
    console.log('>>> Provider Data Entities = ', activeProvider?.entities)

    // Connection Form Labels?
    console.log('>>> Provider Connection Form Labels = ', activeProvider?.getConnectionFormLabels())
     
  }, [activeProvider])

} 

Notice ⚠️
In most cases old code has been commented out to lessen merge conflicts with other in-flight development tickets, more code cleanups will be performed in a future PR. Additionally, legacy component files not actively being used may still reference the old @data/Providers.js constant, hence they won't be updated.

Milestone

v0.15.0

Local Plugin Registry

The local registry defines plugins that are bundled with DevLake. Users creating "Private" local plugins may add and register plugins with the local registry.

API Plugin Registry (Future Feature)

The api registry allows plugins to be defined by the backend and plugins can be fetched dynamically and merged with the local registry. FEATURE TBD

  • Create Dynamic / Live Plugin Registry (Allow Plugins to load via Backend API)

Plugin Registration Schema (JSON)

{
  "id": "gitlab",
  "type": "integration",
  "enabled": true,
  "multiConnection": true,
  "isBeta": false,
  "isProvider": true,
  "name": "GitLab",
  "icon": "src/images/integrations/gitlab.svg",
  "private": false,
  "connection": {
    "authentication": "token",
    "fields": {
      "name": { "enable": true, "required": true, "readonly": false },
      "endpoint": { },
      "proxy": { },
      "token": { },
      "rateLimitPerHour": { }
    },
    "labels": {
      "name": "Connection Name",
      "endpoint": "Endpoint URL",
      "proxy": "Proxy URL",
      "token": "Access Token",
      "rateLimitPerHour": "Rate Limit (per hour)"
    },
    "placeholders": {
      "name": "eg. GitLab",
      "endpoint": "eg. https://gitlab.com/api/v4/",
      "proxy": "eg. http://proxy.localhost:8080",
      "token": "eg. ff9d1ad0e5c04f1f98fa",
      "rateLimitPerHour": "1000"
    },
    "tooltips": {}
  },
  "entities": ["CODE", "TICKET", "CROSS"],
  "transformations": {
    "default": {
      "productionPattern": "",
      "deploymentPattern": ""
    }
  }
}

Does this close any open issues?

#2882

Screenshots

[<PENDING>]
Screen Shot 2022-10-20 at 2 36 25 PM
Screen Shot 2022-10-20 at 2 36 34 PM

Screen Shot 2022-10-20 at 12 39 16 PM

Screen Shot 2022-10-20 at 12 37 24 PM

@e2corporation e2corporation self-assigned this Aug 30, 2022
@e2corporation e2corporation added the component/config-ui This issue or PR relates to config-ui label Aug 30, 2022
@e2corporation e2corporation linked an issue Aug 30, 2022 that may be closed by this pull request
8 tasks
@e2corporation e2corporation added pr-type/refactor This PR refactors existing features pr-type/feature-development This PR is to develop a new feature labels Aug 30, 2022
@tk103331
Copy link
Contributor

tk103331 commented Aug 31, 2022

Wouldn't it be better to define the properties of the field like this:

...
{
    "authentication": "token",
    "fields": [
        {
            "name": "name",
            "label": "Connection Name",
            "placeholder": "eg.GitLab",
            "enable": true,
            "required": true,
            "readonly": false
        },
        {
            "name": "endpoint",
            "label": "Endpoint URL",
            "placeholder": "eg. https://gitlab.com/api/v4/"
        },
        {
            "name": "token",
            "label": "Access Token",
            "placeholder": "eg. ff9d1ad0e5c04f1f98fa"
        }
    ]
}
...

This has at least the following benefits:

  • Focus on all properties of a field when defining fields, rather than defining them in scattered places.
  • The form of the fileds array can solve the sorting problem of the fields, and the way of using the JSON object is likely to cause the problem of the traversal order.

I do see your point, I wouldn't say it's scattered, sorting of the fields is not something needed or relevant, the order of the fields in the connection form follows a natural order starting with Name, ending with Rate Limit.

In addition, do you need to add some other attributes to the field, such as

  • showType, which represents front-end display components, such as input, select, checkbox, etc.
  • validation, which represents standard validation rules, including regular validation, length validation, etc.
  • multiple, which means that it is a multiple format, such as the PAT of the GitHub plugin, you can fill in multiple.

I see the point, and we can certainly evolve the configuration to let validation rules be defined here but it's not needed right now. Instead of showType we would just use type (string|number|object) etc and add a component prop to specify a form control that's to be used for render.

@likyh
Copy link
Contributor

likyh commented Aug 31, 2022

Wouldn't it be better to define the properties of the field like this:

...
{
    "authentication": "token",
    "fields": [
        {
            "name": "name",
            "label": "Connection Name",
            "placeholder": "eg.GitLab",
            "enable": true,
            "required": true,
            "readonly": false
        },
        {
            "name": "endpoint",
            "label": "Endpoint URL",
            "placeholder": "eg. https://gitlab.com/api/v4/"
        },
        {
            "name": "token",
            "label": "Access Token",
            "placeholder": "eg. ff9d1ad0e5c04f1f98fa"
        }
    ]
}
...

This has at least the following benefits:

  • Focus on all properties of a field when defining fields, rather than defining them in scattered places.
  • The form of the fileds array can solve the sorting problem of the fields, and the way of using the JSON object is likely to cause the problem of the traversal order.

In addition, do you need to add some other attributes to the field, such as

  • showType, which represents front-end display components, such as input, select, checkbox, etc.
  • validation, which represents standard validation rules, including regular validation, length validation, etc.
  • multiple, which means that it is a multiple format, such as the PAT of the GitHub plugin, you can fill in multiple.

There is some information in this issue #2846

I think so.

@likyh
Copy link
Contributor

likyh commented Aug 31, 2022

No matter how to do it, it looks like we must use one config to control all the logic. I think #2862 「delete ProviderIcons and ProviderFormPlaceholders and replace them with one config ProviderConfigMap. (integrationsData will refactor in future)」 and 「replace some switch(provider.id)/[xxx,yyy].includes(provider.id)/[jira,github] with more common code.」 are necessary. Why you hate it so much?

@e2corporation
Copy link
Contributor Author

e2corporation commented Aug 31, 2022

No matter how to do it, it looks like we must use one config to control all the logic. I think #2862 「delete ProviderIcons and ProviderFormPlaceholders and replace them with one config ProviderConfigMap. (integrationsData will refactor in future)」 and 「replace some switch(provider.id)/[xxx,yyy].includes(provider.id)/[jira,github] with more common code.」 are necessary. Why you hate it so much?

You go through all the trouble of introducing another foreign ProviderConfigMap which is a hack workaround to stuff all the needed props into one place, add a note that you have to refactor it more in the future -- and you think that's a better solution than the well defined JSON config files? Your solution was more out of desperation than a proper way to evolve the configuration system. I only have 1 commit on this PR -- there are more changes to be added. I already mentioned that the switch statements in connection manager will be consolidated and removed.

@e2corporation
Copy link
Contributor Author

Wouldn't it be better to define the properties of the field like this:

...
{
    "authentication": "token",
    "fields": [
        {
            "name": "name",
            "label": "Connection Name",
            "placeholder": "eg.GitLab",
            "enable": true,
            "required": true,
            "readonly": false
        },
        {
            "name": "endpoint",
            "label": "Endpoint URL",
            "placeholder": "eg. https://gitlab.com/api/v4/"
        },
        {
            "name": "token",
            "label": "Access Token",
            "placeholder": "eg. ff9d1ad0e5c04f1f98fa"
        }
    ]
}
...

This has at least the following benefits:

  • Focus on all properties of a field when defining fields, rather than defining them in scattered places.
  • The form of the fileds array can solve the sorting problem of the fields, and the way of using the JSON object is likely to cause the problem of the traversal order.

In addition, do you need to add some other attributes to the field, such as

  • showType, which represents front-end display components, such as input, select, checkbox, etc.
  • validation, which represents standard validation rules, including regular validation, length validation, etc.
  • multiple, which means that it is a multiple format, such as the PAT of the GitHub plugin, you can fill in multiple.

There is some information in this issue #2846

I thought about this as well, and I agree to a certain extent and may re-configure the json files to be this way. The JSON schema is still being evaluated and worked on. For now it's easier on the user to edit 3 separate objects than 1 config object with 2nd level keys for everything. Additionally, it let's me construct the existing Variable Configurations without having to reduce the object configuration and transform it again into the structure I need.

@e2corporation
Copy link
Contributor Author

A Plugin does not need to be responsible for determining it's own Connection Fields at this time with regards to them being dynamically created, it should be following a standard convention so Each Plugin has to satisfy the current Connection model defined by the backend. The plugin can have dynamic control (enabled state, required state etc) over the standard connection fields that exist currently, which is what this configuration is intended for. Based on a plugin's authentication type that also is a main factor as to what main fields are required for the connection.

In the future the plugin can perhaps have more control and custom rendering options for the field. This is something to be discussed in more detail.

@likyh
Copy link
Contributor

likyh commented Aug 31, 2022

It's the first PR and ProviderConfigMap can be replaced with JSON or API requests at any time.

In addition, my two PRS may not be merged. But please don't create PR so big. It's an open-source project.

@e2corporation
Copy link
Contributor Author

It's the first PR and ProviderConfigMap can be replaced with JSON or API requests at any time.

In addition, my two PRS may not be merged. But please don't create PR so big. It's an open-source project.

ProviderConfigMap is short term hack. Had you invested time in discussing with me what your intentions were you could have saved yourself a lot of time, Instead you choose to refactor on your own and make your own decisions, and create a PR with changes that go against my design strategy.

@e2corporation
Copy link
Contributor Author

@likyh @tk103331 I'll be making more commits as I continue ideating on the Plugin Registry concept. In the mean time, I'm fine with both of you contributing thoughts & ideas to this effort, that are hopefully inline with this strategy.

@tk103331 Feel free to comment on more suggestions/questions or issues for the Plugin Schema as it's being developed, some option configurations can be for a future need as well. The important thing is to get an essential schema for the short-term without overcomplicating it. You will also be able to add commits if you are wanting to contribute.

@likyh When I'm done with a few more commits, I'll tag you to adapt some of your valid refactor changes from your closed PR, such as the Dynamic Menu from new $Integrations var. There are also areas of the useIntegrations hook you can extend, such as the validation handler to verify a plugin's object properties meets Schema specifications, as well as working on live API registry.

@e2corporation e2corporation changed the title feat: setup configuration-ui plugin registry feat: setup configuration-ui plugin registry | Draft Sep 1, 2022
@e2corporation e2corporation changed the title feat: setup configuration-ui plugin registry | Draft feat: setup configuration-ui plugin registry Draft Sep 1, 2022
@tk103331
Copy link
Contributor

tk103331 commented Sep 2, 2022

@e2corporation Sorry, I didn't see what you edited in my reply two days ago.

I do see your point, I wouldn't say it's scattered, sorting of the fields is not something needed or relevant, the order of the fields in the connection form follows a natural order starting with Name, ending with Rate Limit.

What I mean is to spread the multiple information (name, label, placeholder) of a field to be defined in multiple places instead of using only one object.
Regarding field sorting, in general, we will use the code writing order. However, if fileds is data requested from the backend. Possibly, the backend may use the map data structure (because the field is not fixed and cannot use struct), which will cause problems due to the uncertainty of the map traversal order. Of course, we can provide some instructions or helpers to avoid this problem, but it really depends on how the backend people write the code.

I see the point, and we can certainly evolve the configuration to let validation rules be defined here but it's not needed right now. Instead of showType we would just use type (string|number|object) etc and add a component prop to specify a form control that's to be used for render.

Yes, the name component would be more appropriate, the name showType is a premature suggestion.

@e2corporation e2corporation force-pushed the jc-2882-plugin-registry branch from f8e9dce to 74d68a2 Compare September 9, 2022 04:01
@likyh
Copy link
Contributor

likyh commented Oct 12, 2022

Is this PR will continue now?

@e2corporation
Copy link
Contributor Author

Is this PR will continue now?

I will be refreshing this soon, ideally other refactor PRs need to be completed or halted while I get this initial version of dynamic config ready.

@e2corporation e2corporation force-pushed the jc-2882-plugin-registry branch 3 times, most recently from e176241 to 573911a Compare October 20, 2022 15:51
@e2corporation e2corporation force-pushed the jc-2882-plugin-registry branch from 4988aa0 to 51a9d3a Compare October 25, 2022 14:36
@e2corporation e2corporation added this to the v0.15.0 milestone Oct 26, 2022
@e2corporation e2corporation force-pushed the jc-2882-plugin-registry branch from 287be2f to 07ff743 Compare October 27, 2022 16:21
@e2corporation e2corporation force-pushed the jc-2882-plugin-registry branch from e49c9ba to 1203474 Compare October 28, 2022 14:32
@e2corporation
Copy link
Contributor Author

e2corporation commented Oct 28, 2022

@likyh @mintsweet All notable code review feedback items have been resolved, any additional cleanups will be deferred to a subsequent PR.

@likyh likyh merged commit c215d73 into apache:main Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/config-ui This issue or PR relates to config-ui pr-type/feature-development This PR is to develop a new feature pr-type/refactor This PR refactors existing features priority/high This issue is very important

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature][Config-UI] Create Plugin Registry

4 participants