Move to ConnectorType.configuration #4557

lasryaric · 2024-04-03T15:23:03Z

Description

Task: https://github.com/dust-tt/tasks/issues/537

The goal of this PR is to introduce a new interface to manage connectors configurations.

ConnectorType now has a configuration?:ConnectorConfiguration field, with ConnectorConfiguration defined like that as of today:

export type ConnectorConfiguration = WebCrawlerConfigurationType;

and will be defined like that in the future:

export type ConnectorConfiguration = WebCrawlerConfigurationType | SlackConfigurationType | GoogleDriveConfigurationType | ...

There is also a new ConnectorsAPI.updateConfiguration(connectorId, configuration:unknown) API.

Getting the configuration is achieved by getting the ConnectorType.

The configuration of the WebCrawler was moved to the new interface as part of this PR, Slack and GoogleDrive will come later.
The single key configuration system (/connectors/:connector_id/config/:config_key) is now deprecated and will be removed in the future.

This is a big PR, I did not find a way to slice it in smaller pieces. This would have involved keeping an over complicated interface on the connector side just to split the PR, and it was nearly impossible to do it because the types for the interface changed a lot.

Risk

Deploy Plan

lasryaric · 2024-04-03T15:55:08Z

connectors/src/api/update_connector.ts

-      }
-    }
-  })();
+  const connectorUpdater = UPDATE_CONNECTOR_BY_TYPE[connector.type];


Completely removed everything related to configuration from the /update endpoint.

lasryaric · 2024-04-03T15:56:00Z

connectors/src/connectors/index.ts

@@ -298,6 +306,35 @@ export const GET_CONNECTOR_CONFIG_BY_TYPE: Record<
  },
 };

+export const SET_CONNECTOR_CONFIGURATION_BY_TYPE: Record<


For now everything is using the old /config/ interface except the webcrawler.

lasryaric · 2024-04-03T15:59:30Z

connectors/src/connectors/webcrawler/index.ts

@@ -39,31 +36,44 @@ import {

 export async function createWebcrawlerConnector(
  dataSourceConfig: DataSourceConfig,
-  urlConfig: CreateConnectorUrlRequestBody
+  connectionId: string,
+  configuration?: unknown


Configuration should be of known type WebCrawlerConfigurationType here to follow the same design as setWebcrawlerConfiguration

front/pages/api/w/[wId]/data_sources/managed.ts

spolu

Generally looks good. I still need another pass of review but left a first batch of comments 🙏

spolu · 2024-04-03T16:22:02Z

front/pages/api/w/[wId]/data_sources/[name]/configuration.ts

+      },
+    });
+  }
+  if (!auth.user()) {


Ducplicate of isBuilder on l62?

spolu · 2024-04-03T16:22:29Z

front/pages/api/w/[wId]/data_sources/[name]/configuration.ts

+    });
+  }
+
+  if (!auth.isBuilder()) {


Maybe we can simply rely on the switch below so duplicate as well?

front/pages/api/w/[wId]/data_sources/managed.ts

types/src/front/lib/connectors_api.ts

types/src/connectors/api_handlers/create_connector.ts

spolu

LGTM

I think we can constraint the updateConnector front API iots back to requiring a connectorId ?

Let's also enforce a whitelist of connectors you can update the configuration of in front, allowing only webcrawler for now 🙏

spolu · 2024-04-04T13:32:16Z

types/src/connectors/api_handlers/update_connector.ts

-    CreateConnectorUrlRequestBodySchema,
-    UpdateConnectorOAuthRequestBodySchema,
-  ]),
+  connectionId: t.union([t.string, t.null, t.undefined]),


Why allow undefined / null here?

I think we want to enforce that update pass a connection as in practice this update really is an update connection only?

And connectorsAPI enforces it right?

spolu · 2024-04-04T13:32:43Z

front/pages/api/w/[wId]/data_sources/[name]/managed/update.ts

@@ -95,7 +95,7 @@ async function handler(
      const connectorsAPI = new ConnectorsAPI(logger);
      const updateRes = await connectorsAPI.updateConnector({
        connectorId: dataSource.connectorId.toString(),
-        params: bodyValidation.right,
+        connectionId: bodyValidation.right.connectionId || undefined,


Should never happen if we fix the iots?

spolu · 2024-04-04T13:39:42Z

front/pages/api/w/[wId]/data_sources/[name]/configuration.ts

+
+      const connectorsAPI = new ConnectorsAPI(logger);
+
+      const updateRes = await connectorsAPI.updateConfiguration({


We likely want to enforce a whiltelist of configration users can update. For now webcrawler (to avoid allowing them to change flags as we move them to configurations)

spolu · 2024-04-04T13:42:11Z

connectors/src/api_server.ts

@@ -146,11 +153,6 @@ export function startServer(port: number) {
    getConnectorConfigAPIHandler
  );

-  app.get(


spolu · 2024-04-04T13:42:50Z

connectors/src/connectors/interface.ts


 import type { DataSourceConfig } from "@connectors/types/data_source_config";

-export type ConnectorCreatorOAuth = (


spolu · 2024-04-04T13:44:04Z

front/pages/api/w/[wId]/data_sources/managed.ts

-
-          break;
-        }
-        case "url": {


spolu · 2024-04-04T13:45:20Z

types/src/front/data_source.ts

@@ -16,19 +16,6 @@ export function isConnectorProvider(val: string): val is ConnectorProvider {
  return (CONNECTOR_PROVIDERS as unknown as string[]).includes(val);
 }

-export const provider2createConnectorType: Record<


spolu

LGTM

Please proceed to extensive testing of connector creation (webcrawler but other as well) + webcrawler updates as this is a rather big change :)

@spolu

Fix managed/update renderConnectorType() Fully working for websites Clean up Cleaning up the createConnector Interface Clean up typing in connectors Removes unknown from connectorsAPI.updateConfiguration Removing unknown Cleaning up following @spolu's comments. Remvoe console.log() Just removing a comment Make connectionId non optional Make sure only webcrawler configuration can be updated by a user

lasryaric · 2024-04-04T14:19:03Z

LGTM
Please proceed to extensive testing of connector creation (webcrawler but other as well) + webcrawler updates as this is a rather big change :)

Already did but will re-do it right now with the final commit.

lasryaric · 2024-04-04T14:56:15Z

@spolu tested the following with the final commit:

Create a folder and upload 20 PDFs.
Connect a new Slack
Revoke my Slack authorization
Update my Slack connector, observed that it works again
Exact same thing with Google Drive
Created a Website
Changed all settings, observed that it worked
Deleted the website
Added another website

All worked well

@spolu

* Tmp commit Fix managed/update renderConnectorType() Fully working for websites Clean up Cleaning up the createConnector Interface Clean up typing in connectors Removes unknown from connectorsAPI.updateConfiguration Removing unknown Cleaning up following @spolu's comments. Remvoe console.log() Just removing a comment Make connectionId non optional Make sure only webcrawler configuration can be updated by a user * Fix typing

lasryaric commented Apr 3, 2024

View reviewed changes

front/pages/api/w/[wId]/data_sources/managed.ts Outdated Show resolved Hide resolved

lasryaric requested a review from spolu April 3, 2024 16:17

spolu reviewed Apr 3, 2024

View reviewed changes

spolu requested changes Apr 4, 2024

View reviewed changes

lasryaric force-pushed the aric-isolate_webcrawler_configuration branch 2 times, most recently from 157ee15 to ec2e4e6 Compare April 4, 2024 14:11

spolu approved these changes Apr 4, 2024

View reviewed changes

lasryaric force-pushed the aric-isolate_webcrawler_configuration branch from ec2e4e6 to b0128d8 Compare April 4, 2024 14:18

Fix typing

3bbfd65

lasryaric merged commit 4295a6c into main Apr 4, 2024
5 checks passed

lasryaric deleted the aric-isolate_webcrawler_configuration branch April 4, 2024 14:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move to ConnectorType.configuration #4557

Move to ConnectorType.configuration #4557

lasryaric commented Apr 3, 2024 •

edited

Loading

lasryaric Apr 3, 2024

lasryaric Apr 3, 2024

lasryaric Apr 3, 2024 •

edited

Loading

spolu left a comment

spolu Apr 3, 2024

spolu Apr 3, 2024

spolu left a comment

spolu Apr 4, 2024

spolu Apr 4, 2024

spolu Apr 4, 2024

spolu Apr 4, 2024

spolu Apr 4, 2024

spolu Apr 4, 2024

spolu Apr 4, 2024

spolu Apr 4, 2024

spolu Apr 4, 2024

spolu left a comment

lasryaric commented Apr 4, 2024

lasryaric commented Apr 4, 2024 •

edited

Loading


		const connectorsAPI = new ConnectorsAPI(logger);

		const updateRes = await connectorsAPI.updateConfiguration({


		import type { DataSourceConfig } from "@connectors/types/data_source_config";

		export type ConnectorCreatorOAuth = (

Move to ConnectorType.configuration #4557

Move to ConnectorType.configuration #4557

Conversation

lasryaric commented Apr 3, 2024 • edited Loading

Description

Risk

Deploy Plan

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lasryaric Apr 3, 2024 • edited Loading

Choose a reason for hiding this comment

spolu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spolu 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

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

spolu left a comment

Choose a reason for hiding this comment

lasryaric commented Apr 4, 2024

lasryaric commented Apr 4, 2024 • edited Loading

lasryaric commented Apr 3, 2024 •

edited

Loading

lasryaric Apr 3, 2024 •

edited

Loading

lasryaric commented Apr 4, 2024 •

edited

Loading