Skip to content

[devbox.json] support env_from dotenv files #2174

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

Merged
merged 2 commits into from
Jun 28, 2024
Merged

Conversation

mohsenari
Copy link
Collaborator

@mohsenari mohsenari commented Jun 26, 2024

Summary

NOTE: Docs update PR will follow.
As per request from users, I added support for env_from field being able to take a path to *.env file and have those env variables available in devbox shell and run.

If a duplicate env variable is specified in both .env and in the "env" section of devbox.json, the env section from devbox.json takes priority and overwrites. This I believe is the correct behavior but I'm open to discussion.

How was it tested?

see test file at testscripts/run/envfrom.test.txt

@mohsenari mohsenari requested a review from mikeland73 June 26, 2024 19:03
@Lagoja
Copy link
Contributor

Lagoja commented Jun 27, 2024

Do .env files (and our parser) also support the following format:

export S3_BUCKET=YOURS3BUCKET
export SECRET_KEY=YOURSECRETKEYGOESHERE

@mohsenari
Copy link
Collaborator Author

mohsenari commented Jun 27, 2024

Do .env files (and our parser) also support the following format:

export S3_BUCKET=YOURS3BUCKET
export SECRET_KEY=YOURSECRETKEYGOESHERE

@Lagoja not common but .env file format seems to have export in front of each line in some cases .
But because .env doesn't have an official standard (although dotenv.org claims to be the creator of dotenv), some others specifications don't support the export.
So ultimately, it's up to us if we want to add it in our parser. Currently, the parser doesn't support it.

@mohsenari mohsenari requested a review from Lagoja June 27, 2024 17:00
@Lagoja
Copy link
Contributor

Lagoja commented Jun 27, 2024

I think I'm ok to add it later, current functionality is probably good for now

@mohsenari mohsenari merged commit 1252033 into main Jun 28, 2024
24 checks passed
@mohsenari mohsenari deleted the mohsen--env-from-dotenv branch June 28, 2024 14:11
return nil, fmt.Errorf("env file does not have a .env extension")
}

file, err := os.Open(c.EnvFrom)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fail if devbox is call from any other directory that is not where the devbox.json is. I don't think it makes sense to make this relative from the working directory, it should be relative from the config file directory.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with this, my project failed due to it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@varunpalekar thanks for reminding on the issue. I made a PR to fix this, once merged it will be available in the next version of devbox.

@JoyceBabu
Copy link
Contributor

If a duplicate env variable is specified in both .env and in the "env" section of devbox.json, the env section from devbox.json takes priority and overwrites. This I believe is the correct behavior but I'm open to discussion.

There are two scenarios that need to be addressed:

  1. Devbox-specific override of project-specific environment variables:
    For example, if I have a .env file containing the default environment variables for the project, the current behavior allows overriding these defaults using the env section in devbox.json.

  2. Overriding devbox defaults on a per-environment basis:
    How can developers override the project defaults on their individual machines? For instance, if I have an environment variable PHP_MODULES_DIR that defaults to a path on Linux, I want to override it on my MacBook. How can I achieve this?

It would be ideal to have a way to override devbox.json itself by creating a devbox.override.json, similar to how composer.override.yaml works in Docker Compose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants