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

#724 Fix Environment Variable Collisions #812

Closed ItsMeBrianD closed 11 months ago

ItsMeBrianD commented 1 year ago

Description

Resolves #724

Checklist

changeset-bot[bot] commented 1 year ago

🦋 Changeset detected

Latest commit: d000ae0318c33768a3bb7f3fb6df32202ffafb5b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 14 packages | Name | Type | | ----------------------------- | ----- | | @evidence-dev/bigquery | Minor | | @evidence-dev/db-commons | Minor | | @evidence-dev/db-orchestrator | Minor | | @evidence-dev/duckdb | Minor | | @evidence-dev/mysql | Minor | | @evidence-dev/postgres | Minor | | @evidence-dev/snowflake | Minor | | @evidence-dev/sqlite | Minor | | evidence-docs | Patch | | @evidence-dev/csv | Patch | | @evidence-dev/mssql | Patch | | @evidence-dev/redshift | Patch | | @evidence-dev/evidence | Major | | evidence-test-environment | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
evidence-development-workspace ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 30, 2023 2:31pm
evidence-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 30, 2023 2:31pm
netlify[bot] commented 1 year ago

Deploy Preview for evidence-development-workspace ready!

Name Link
Latest commit d000ae0318c33768a3bb7f3fb6df32202ffafb5b
Latest deploy log https://app.netlify.com/sites/evidence-development-workspace/deploys/6476086ec4e3330009104a53
Deploy Preview https://deploy-preview-812--evidence-development-workspace.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

archiewood commented 1 year ago

@ItsMeBrianD What does this need? More review?

ItsMeBrianD commented 1 year ago

@archiewood I think one more pass; but everything should be good to go here

archiewood commented 1 year ago

I can test if you need, what's the ideal protocol?

ItsMeBrianD commented 1 year ago

Essentially; the updated docs have all the "correct" environment variables, those should work without any warnings, using "existing" environment variables (e.g. ones that aren't EVIDENCE scoped), should log a warning stating that they are deprecated.

archiewood commented 1 year ago

there are all only used in production though, so I assume I need to test a deployment with different permutations of the above variables them?

And read the deployment logs

archiewood commented 1 year ago

Deprecated Vars

Okay so using:

image

I'm getting:

image

Should I also be getting warnings about?

I notice that DUCKDB_FILENAME is not marked as deprecated, so perhaps this is as you intended?

Supported Vars

When using:

I get no warnings, and a successful build, as expected

ItsMeBrianD commented 12 months ago

DUCKDB_GITIGNORE_DUCKDB

I don't know if this environment variable is used anywhere; I searched through the project and didn't find anything.

DUCKDB_FILENAME

This was not flagged as deprecated; we can change that if we'd like to

ItsMeBrianD commented 12 months ago

there are all only used in production though, so I assume I need to test a deployment with different permutations of the above variables them?

It could be done this way, or you can build locally;

If you create a .env file with export SOME_ENV_VAR=SOME_ENV_VALUE, then run . .env, that will load the environment variables, you should be able to use npm run build and it will behave like a production build

archiewood commented 11 months ago

This was not flagged as deprecated; we can change that if we'd like to Not sure it's that important

I'm done on this from my side