evidence-dev / evidence

Business intelligence as code: build fast, interactive data visualizations in pure SQL and markdown
https://evidence.dev
MIT License
3.44k stars 167 forks source link

Support latest duckdb #885

Closed hughess closed 11 months ago

hughess commented 1 year ago

Bug Description

The Evidence duckdb connector isn't able to read the newest version of duckdb files. I assume we need a general solution here to handle future cases of this as well - where we have a version that Evidence is supporting, but users who want to upgrade to the newest duckdb version can do that and use it in Evidence.

Severity

The dbt jaffle shop tutorial relies on a newer version of duckdb and is currently not working with evidence: https://github.com/dbt-labs/jaffle-shop-template

mcrascal commented 12 months ago
csjh commented 12 months ago
image

Sadly looks like it isn't backwards compatible, but gives a helpful error message

archiewood commented 12 months ago

@csjh We'll also need to update the duckdb file we ship with the template project. I can take you through the process needed to generate it!

csjh commented 12 months ago

I think I have that handled but I'm not sure if there's a good solution for the DDB override short of telling them to do npm i duckdb-async@<version in the error message>. Could also do a fancy solution that wouldn't need any user override with a storage header read to check version and automatically install the proper version. Should work fine as long as there's no change to the part of the library we interact with.

archiewood commented 12 months ago

DuckDB's filetypes are moving very fast, and breaking their API a lot.

I think most people who use DuckDB know this, so I'd be inclined to throw the error and let them sort out their file version.

Many people are making their duckdb files on the fly I imagine, so I don't think this is a huge deal.

ie We upgrade the duckdb-async connector to 8.0.0 and roll with it.

0.7.1 is dead, long live 0.8.0

RandomFractals commented 11 months ago

On a related note, I ran into this issue while using Evidence template to create new project. Mainly because that template app uses sample needful_things.duckdb created with DuckDB v0.7.1 and evidence libraries were updated to support DuckDB v0.8.0.

I managed to work around it by adding duckdb-async v0.7.1 override to the package.json in new Evidence project created from a template to get an evidence app render pages without errors.

  "overrides": {
    "jsonwebtoken": "9.0.0",
    "trim@<0.0.3": ">0.0.3",
    "sqlite3": "5.1.5",
    "duckdb-async": "0.7.1"
  }

In order to patch template project for use with the latest versions of @evidence-dev/* libraries, either needful_things.duckdb should be recreated with DuckDB v0.8.0, or that duckdb-async v0.7.1 override needs to be added for all projects created with that sample DuckDB file.

Also, would be great if template project did not require manually adding that data source, and included that sample DuckDB config in .evidence/template/evidence.settings.json without npm run dev overwriting it on the first run.

I suggest we add that evidence.settings.json config to the template project repo and ensure we don't delete it during project dev or production build. Then we could use template project as a starter without any manual config updates.

.evidence/template/evidence.settings.json:

{
  "database": "duckdb",
  "credentials": {
    "filename": "needful_things.duckdb",
    "gitignoreDuckdb": null
  }
}

Also, do we need sqlite3 dependency overwrite? Does evidence app currently use it as embedded DB? If not, maybe we should remove it from the template project package.json.

Winterhart commented 11 months ago

The goal is to keep the template "enterprise friendly". Those overrides were put in place to patch CVE as we want to maintain the template at 0 CVE.

They probably need to be updated by now.

At some point, it would be nice to have a bot that do the patching.

RandomFractals commented 11 months ago

@Winterhart What do you mean by CVE? We recently added new Create Project from Template feature to Evidence vscode extension that we plan to release this week. That feature uses template project to start new Evidence app. The changes I listed above are required for the template project to build and run after copying without requiring any extra manual config updates.

archiewood commented 11 months ago

@RandomFractals CVE: https://www.redhat.com/en/topics/security/what-is-cve It's the warning messages that get flagged in the terminal after you npm i

re duckdb:

RandomFractals commented 11 months ago

@Winterhart If you'd like to support multiple versions of DuckDB in the template project., creating tagged releases of that project could help us and enterprise users who prefer more stable older versions of DuckDB to use the same sample app template. That would ensure low CVE for enterprise projects created from a template, and allow new users to use the latest DuckDB version.

csjh commented 11 months ago

On a related note, I ran into this issue while using Evidence template to create new project. Mainly because that template app uses sample needful_things.duckdb created with DuckDB v0.7.1 and evidence libraries were updated to support DuckDB v0.8.0.

How were you getting this error? The duckdb adapter is pinned to 0.7.1 until #891 is pushed

RandomFractals commented 11 months ago

@csjh I must have gotten it after updating your packages to latest in the template project ...

* Evidence node modules to update to the latest version.
 */
const evidencePackages: string[] = [
  '@evidence-dev/evidence@latest',
  '@evidence-dev/preprocess@latest',
  '@evidence-dev/components@latest'
];