toucan-connectors icon indicating copy to clipboard operation
toucan-connectors copied to clipboard

Split connector's config model from connector's logic

Open davinov opened this issue 4 years ago • 1 comments

With the arrival of new connectors, like the new google sheets one (#202), I see new uses of connector's properties:

  • some "flagging" attributes: like auth_flow: "oAuth2". These attributes are useful to indicate behavior changes for some groups of connectors.
  • some "internal" attributes: like secrets. These attributes are necessary to run a query, but should not be filled directly by a user when configuring a connector. Instead, they are populated by the server that makes use of the connector. Both have in common that they should not appear in the schema (which will be used to display a form) to create or edit a connector.

For the first type, I tested the ClassVar annotation and making the attribute private (prepended with _). It does prevent to display the info in the schema, but also hide the info when exporting the document as dict. Therefore, a front-end would not be able to have this information, and for example open a consent screen for connectors having the authFlow property to "oAuth2". This starts to show that we want our users to configure, and what we want to export as a connector could sometimes becomes a bit different.

For the second type, private attributes works well combined with setters. But it's still a bit of tricky logic to have to think of making them private to exclude them from being stored.

What I suggest is to separate the ConnectorConfig from the Connector itself. ConnectorConfig is a good application of pydantic's BaseModel. It's something that will comes from user input and stored in a DB or a config file. It should be validated. Connector stores a lot of logic, and this logic needs the configuration object, but also more and more other info.

Users provides the ConnectorConfig, which is stored in DB. Server creates a Connector from a ConnectorConfig and other relevant info. Serve can serialize a Connector and send it, with its config and other relevant info if necessary.

What are your opinion on this?

Related links: https://github.com/samuelcolvin/pydantic/issues/655 specially this comment about God-like objects

davinov avatar Sep 06 '20 07:09 davinov

I think it's a good idea to separate out the configuration but not sure in a way about the database from the point of view that so far no connector sends queries to a database/our database. Currently, it's only our server (private repo) that handles communication with a database. We lose the abstraction that the connectors project so far has from any of our other projects if we store the configuration in the database.

I'm for keeping them in configuration files and in my mind that's what we should ultimately do (for eg. in JSON files). However, this would only be great for static properties like baseroute. If we have to make a new file for every set of secrets we'd want to store, that would become problematic.

testinnplayin avatar Sep 15 '20 12:09 testinnplayin