Conversation
| def run_sql( | ||
| self, file_config: T, sql: str, params: list[Any] | None = None, read_only: bool = True | ||
| ) -> SqlExecutionResult: | ||
| return self._introspector.run_sql( |
There was a problem hiding this comment.
Nit: This method doesn't really belong in the introspector
I'm not sure it's worth it to take it out though: you'd need to refactor a bit to have a class executing SQL (including our SQL introspection queries) that is used by the Introspector
|
|
||
|
|
||
| @runtime_checkable | ||
| class SqlRunnablePlugin[T](BuildDatasourcePlugin[T], Protocol): |
There was a problem hiding this comment.
This feel like it should be its own independent protocol rather than extending BuildDatasourcePlugin
And plugins would keep on implementing BuildDatasourcePlugin but they would also implement SqlRunnablePlugin on top
I'm not fully sure how well it would work with the generics since we would want it to be the same for both the BuildDatasourcePlugin and the SqlRunnablePlugin 🤔
There was a problem hiding this comment.
Or maybe it would be easier for the run_sql to always be part of BuildDatasourcePlugin but with a default implementation throwing an exception. And instead of casting, we're always calling the method and catching the NotSupported exception
| datasource_ids: list[DatasourceId] | None = None, | ||
| ) -> list[ContextSearchResult]: | ||
| """Search in the avaialable context for the closest matches to the given text. | ||
| """Search in the available context for the closest matches to the given text. |
| def _apply(plugins_map): | ||
| mocker.patch( | ||
| "databao_context_engine.plugin_loader.load_plugins", | ||
| new=lambda: plugins_map, |
There was a problem hiding this comment.
I'm pretty sure the DatabaoContextPluginLoader takes an optional map of plugins (that was added specifically for tests to avoid patching)
There was a problem hiding this comment.
Nit: Since you're testing the databao_context_engine directly, I would move this file directly in tests
2 participants
No description provided.