Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect precedence overwriting env vars with dotenv #2267

Closed
craigbeck opened this issue Mar 3, 2021 · 4 comments · Fixed by #2307
Closed

Incorrect precedence overwriting env vars with dotenv #2267

craigbeck opened this issue Mar 3, 2021 · 4 comments · Fixed by #2307
Assignees
Labels
bug Something isn't working

Comments

@craigbeck
Copy link

craigbeck commented Mar 3, 2021

Describe the bug

Recently did a deployment of CubeJs instance and instance attempted to connect to our Postgres instance but could not as it was using the .env file values rather than the supplied environment variables.

To Reproduce

Expected behavior

  • expected previously working configuration to continue to work
  • expected dotenv variable precedence to behave in the same expected way

Screenshots
n/a

Version:
@cubejs-backend/server@0.26.12

Additional context

  • .env file supplied has values for local development
  • in GCP we use the app.yaml files to set environment specific values that take precedence over anything in .env

Here is where you dotenv is setup to overwrite existing env variable values: https://github.com/cube-js/cube.js/blob/d9f5154e38c14417aa332f9bcd3d38ef00553e5a/packages/cubejs-server/src/server.ts#L24-L27

This is the change in the CubeJs fork of @cubejs-backend/dotenv that allowed this: cube-js/dotenv@e681675

@ovr
Copy link
Member

ovr commented Mar 3, 2021

Hello @craigbeck ,

Do you have any reasons to use both .env & app.yml files together in production instead of using only app.yml?

Thanks

@craigbeck
Copy link
Author

.env file is used locally for ease of setup for developers, but we also depend on the well understood dotenv behavior of being able to override values with environment variables for various uses e.g. ad hoc configuration changes for testing etc

OVERRIDE_THIS_DEFAULT_FROM_ENV=true yarn start

A couple of our vars are common across multiple instances, and we use the app.yaml to override in the cases where this isn't the case.

I have extracted these and distributed to our app.yaml files, and now ignoring our .env file from the docker image as a work around.

The other issue is this was still an unexpected change that negatively impacts developer experience, one of a few issues I've found in the configuration space lately.

@craigbeck
Copy link
Author

@ovr what is the reasoning behind change the widely understood, expected behavior of dotenv?

@ovr
Copy link
Member

ovr commented Mar 3, 2021

The other issue is this was still an unexpected change that negatively impacts developer experience

Very sad to hear that you were affected by this change.

one of a few issues I've found in the configuration space lately.

Feel free to create an issue or write in our Slack to indicate a problem and make Cube.js better.

what is the reasoning behind change the widely understood, expected behavior of dotenv?

Cube.js provides an ability to do reload the configuration with SIGUSR1 https://github.com/cube-js/cube.js/blob/master/packages/cubejs-server/src/server/container.ts#L325

I will talk with our team to find a proper solution how it should work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants