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

Environment support #125

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

flowchartsman
Copy link

This PR addresses support for cumulative environment modification with .env files as well as OS environment variable expansion in commands and the indir directive.

@joncalhoun
Copy link
Collaborator

I haven't really looked at the code closely, so can you explain in more detail what this is doing?

Is it expanding ENV variables declared in a .env file prior to running commands?
Does this happen for all prep commands?
Does it do anything else?
What are the benefits of this over a developer using something like godotenv on their own?

I'm hesitant to add this as it seems like more things to manage and document. It could confuse people who aren't expecting modd to do anything with a .env file. And also because I barely have time to handle my own workload, let alone manage more PRs here 🤣

@flowchartsman
Copy link
Author

flowchartsman commented Mar 8, 2023

Sure thing! Apologies in advance for some of the diffs, some of it is editor styling, and a chunk of it is updating deps. Honestly, I thought the project might be set for archival, so much of this is to support my own usage, and the PR is kind of an "in case this is still maintained and you can get something useful from it" sort of situation. I understand the demands of work/open source balance. Think of this as a WIP to cherry pick from or something I can trim down if you like.

So, for my use-case, I use modd for local development on a single-module project with a few different microservices, using containers for my downstream dependencies. My modd.conf spins up the services I want to live code on, rebuilding them when I need. So far so good. The services configure themselves using the environment, and it's handy to be able to change these values in one place. I can't change them in the modd.conf, so I thought it would be convenient to have them in .env files. Also, sometimes I want to try out a couple different variations, and it's easy to add a new .env file and temporarily comment out a single line in the modd conf without any intention of checking anything in. It also lets me keep the testing/live coding environment out of the repository.

The PR also adds the ability to expand environment variables in the commands issued by modd itself, such as daemon prep and indir, which I found useful for building things into $TMPDIR among others.

With regards to confusing people, that's 100% your call to make as the maintainer, since it's your project and your vision. For my part, I was confused that modd did not provide any functionality with regards to environment. As I saw it, anything that controls services with a config file benefits from some sort of environment handling, even if it's just a plain env directive, and I can certainly see users wanting to have more than one conf file that doesn't change more than environment settings for the different programs it rebuilds. Using .env files was expedient for me, which is why I didn't implement a plain env keyword, though I plan to, and I can see them working in concert to have overrides for single values in the .env files. It's just more flexible this way. If you disagree, well I have my fork, and I won't lose any sleep over it.

If you think some part of this might be useful, I am happy to discuss, amend, withdraw or reissue. It's your show!

@flowchartsman
Copy link
Author

flowchartsman commented Mar 8, 2023

Is it expanding ENV variables declared in a .env file prior to running commands?

The env file stuff doesn't expand anything. It's simple KV environment variables that it adds into the environment of subprocess calls, such as daemons, for which it is intended.

Does this happen for all prep commands?

Edit: yes, actually, I believe it does in the current implementation. At least for prep commands within that block. Env is added to the block when the directive is encountered. It doesn't need to do it this way, and can be modified to be cumulative and additive if necessary. Or it can apply only to the daemons, which is its intended use in the first place.

cmd/service1/**/*.go {
    prep +onchange: go build -o ./.devbuild ./cmd/service1
    envfile: dev/config/env
    envfile: dev/config/env.private
    daemon +sigterm: ./.devbuild/service1
}

cmd/service2/**/*.go {
    prep +onchange: go build -o ./.devbuild ./cmd/service2
    envfile: dev/config/another.env.maybe
    indir: ./.devbuild
    daemon +sigterm: service2
}

Does it do anything else?

It expands environment variables in commands and indir. This is orthogonal to the .env file functionality. Sorry for the conflation. They're separate features.

What are the benefits of this over a developer using something like godotenv on their own?

This allows services to continue being configured the way they would in a container (if they're using the environment). godotenv requires the service to have a dependency and to be aware of the .env file. Why bother with that just for testing and development, when that's usually the purview of the orchestrator?

@joncalhoun
Copy link
Collaborator

I don't see me really diving into this PR for at least a couple months. I know that sounds nuts, but I need to prioritize a few other projects and I really don't have too much time to manage this aside from trying to keep it working. Just letting you know so you don't take it the wrong way 😄

@flowchartsman
Copy link
Author

flowchartsman commented Mar 21, 2023 via email

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

Successfully merging this pull request may close these issues.

2 participants