Add support for environment variables with default values in fig.yml #845

relwell · 2015-01-15T00:18:57Z

This change resolves #495 by introducing a Config class inside the cli
library. The Config class is responsible for loading a .yml file and
evaluating its contents for environment variables. The Config immediately
reloads the data as a dictionary, which reduces compatibility issues within
previously written code. This behavior also prevents race conditions
caused by changes to the environment during long-running fig builds.

This change is valuable because it allows for a greater degree of
automation and variability when working with multiple environments
or deployment types that use identically built containers. We
currently have to define separate services for each degree of
variability we want to account for.

Future steps for this change may include introducing template files,
or adding access and manipulation APIs directly so that we can
perpetuate the config class everywhere we are currently using
bare dictionaries.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

Signed-off-by: Robert Elwell robert.elwell@gmail.com
Sponsored by: Lookout, Inc.

dnephin · 2015-01-15T00:33:19Z

docs/yml.md

@@ -10,6 +10,14 @@ Each service defined in `fig.yml` must specify exactly one of `image` or `build`

 As with `docker run`, options specified in the Dockerfile (e.g. `CMD`, `EXPOSE`, `VOLUME`, `ENV`) are respected by default - you don't need to specify them again in `fig.yml`.

+Fig supports using environment variables with optional backoffs as values. The accepted patterns are as follows:


Minor terminology issue: I would expect "default" here instead of "backoff"

Noted, will fix

dnephin · 2015-01-15T00:55:10Z

Thanks for the PR!

I like the direction this is taking, I've left a few comments. I suspect others will be interested in this as well.

relwell · 2015-01-15T01:28:34Z

Glad to hear it! Do you prefer an additional commit or an amended commit for revisions?

thaJeztah · 2015-01-15T18:26:50Z

docs/yml.md

+Fig supports using environment variables with optional defaults as values. The accepted patterns are as follows:
+
+```
+${ENVIRONMENT_VARIABLE:backoff}


s/backoff/default/

aanand · 2015-01-15T18:31:06Z

Thanks!

I'm similarly wary of regular expressions, but if there's no practical alternative then fine. In either case we're going to need an extensive set of unit tests to cover edge cases and such.
How do I escape the dollar-curly-brace syntax, if I actually want the string ${FOO} to appear?
A single, squashed commit is preferred.

thaJeztah · 2015-01-15T18:32:53Z

Good points by Aanand.

I added a couple of nits while glancing over the PR.

Thanks very much, this looks like a very useful feature that I'll definitely use!

relwell · 2015-01-15T21:19:33Z

@aanand I've added in support to escape the text as "${FOO}". Please refer to (the following test)[https://github.com//pull/845/files#diff-a1643645d7f79dbd5684ed0631ab1066R50] and let me know if you think anything is missing here.

thaJeztah · 2015-01-15T21:24:52Z

fig/cli/config.py

+
+def from_yaml_with_environment_vars(yaml_filename):
+    """
+    Resolves enviornment variables in values defined by a YAML file and transformed into a dict


Typo; enviornment

relwell · 2015-01-16T17:49:20Z

I've added some basic regex tokenization support that will allow us to provide multiple environment variables per line, and accompanying tests.

relwell · 2015-02-06T16:32:35Z

@aanand , it's been 22 days since #846 was opened, and not much has been decided. The community seems to be heavily in favor of delivering a solution to #495 sooner rather than later.

If you take #426 into account, there has been an open community request for a feature like this for over 120 days.

Since this seems to be close to the direction we want to take, and there have been no additional revision requests, I'd appreciate a merge on this so that we can show some progress on this issue. If you can't merge now, I think we'd all appreciate some kind of timeline or feedback, since we haven't seen much activity from the admins on this issue for the last couple of weeks.

@dnephin

This change resolves docker#495 by introducing a Config class inside the cli library. The Config class is responsible for loading a .yml file and evaluating its contents for environment variables. The Config immediately reloads the data as a dictionary, which reduces compatibility issues within previously written code. This behavior also prevents race conditions caused by changes to the environment during long-running fig builds. This change is valuable because it allows for a greater degree of automation and variability when working with multiple environments or deployment types that use identically built containers. We currently have to define separate services for each degree of variability we want to account for. Future steps for this change may include introducing template files, or adding access and manipulation APIs directly so that we can perpetuate the config class everywhere we are currently using bare dictionaries. Added in revision requests from @dnephin: * Terminology fix in docs/yml.md * Refactored config class into a series of functions * Updated tests to reflect functional interpolation * Improved forward compatibility Added in revision requests from @thaJeztah * Terminology fixes Added in revision requests from @aanand * Support for escaping Added in revision requests from @thaJeztah * Support for multiple variables per line Developer's Certificate of Origin 1.1 By making a contribution to this project, I certify that: (a) The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file; or (b) The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file; or (c) The contribution was provided directly to me by some other person who certified (a), (b) or (c) and I have not modified it. (d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved. Signed-off-by: Robert Elwell <robert.elwell@gmail.com> Sponsored by: Lookout, Inc.

relwell · 2015-02-06T16:59:08Z

FYI: I've updated the PR to support the new docker compose format.

relwell · 2015-02-10T17:12:28Z

@aanand @thaJeztah @dnephin would it be possible to get some status on this PR, and any outstanding issues that may need to be addressed to get it merged in?

thaJeztah · 2015-02-10T17:55:06Z

Unfortunately, I'm not a maintainer, so I cannot give you an update. I like this feature and certainly hope this gets approved/merged. It may need a final review to check if everything is okay to merge.

aanand · 2015-02-11T18:43:19Z

I'm going to review this soon - we've a lot on our plate with the impending Compose release. I'd like to get this merged soon after that release, but it needs thorough inspection and fuzz-testing.

hawknewton · 2015-03-04T21:05:37Z

For what it's worth I find environment-specific configuration values shouldn't have defaults.

This usually goes one of two ways: either the developer enters development-time defaults and forgets to update them for production (?!) or the defaults are for production and you have to override them in your development/QA environments. Having your dev/QA deploy jobs diverge from your production deploy jobs is a clear anti-pattern not to mention the terrifying possibility that somebody forgets to override the production database information for a QA deploy.

I'm super-interested in coming up with a mechanism by which a given profile could be activated as defined in the the yaml that would tailor the application for a given environment or class of environment. When in QA mode I might use a lower TTL to facilitate testing. When in development mode maybe I force the developer to provide database connection information via environment variables, etc.

Anyway, just my two cents.

torbjornvatn · 2015-04-01T08:18:28Z

Any news on when we can expect this PR to appear in a release. It's a feature I really need to make make docker-compose scheme to work

hadim · 2015-04-07T17:45:28Z

Any news here ?

relwell · 2015-04-23T18:11:02Z

Is there a plan for integrating this pull request, ever?

I'm happy to rebase and re-push, but I'm not going to bother if you guys don't have a plan to integrate these changes.

My ask is that if there is not a plan to integrate these changes, that you reject the CR, and close #495 as a "won't fix" feature request.

aanand · 2015-04-30T17:55:54Z

I've put the current state of my thinking at #1377. Summary: this is the best design so far, but the implementation isn't there yet.

relwell · 2015-04-30T18:02:45Z

I'll leave you guys to worry about handling this then. I'm tired of wasting my time with this.

dnephin reviewed Jan 15, 2015
View reviewed changes

relwell force-pushed the master branch 2 times, most recently from 9cb07d1 to 30c589c Compare January 15, 2015 17:56

aanand mentioned this pull request Jan 15, 2015

Next-generation configuration format #846

Closed

thaJeztah reviewed Jan 15, 2015
View reviewed changes

relwell force-pushed the master branch from 30c589c to ab81a7a Compare January 15, 2015 20:59

thaJeztah reviewed Jan 15, 2015
View reviewed changes

relwell force-pushed the master branch from ab81a7a to 6588a89 Compare January 16, 2015 17:48

relwell force-pushed the master branch from 6588a89 to 061460d Compare February 6, 2015 16:58

relwell mentioned this pull request Feb 10, 2015

Add support for encrypted environment variables #909

Closed

relwell mentioned this pull request Feb 10, 2015

Feature/env #76

Closed

aanand changed the title ~~Add support for environment variables with backoff values in fig.yml~~ Add support for environment variables with fallback values in fig.yml Feb 11, 2015

aanand changed the title ~~Add support for environment variables with fallback values in fig.yml~~ Add support for environment variables with default values in fig.yml Feb 11, 2015

aanand mentioned this pull request Feb 19, 2015

Proposal: Types, imports and external services #988

Closed

thaJeztah mentioned this pull request Mar 15, 2015

Provide a way to override environment variables #1089

Closed

thaJeztah mentioned this pull request Apr 12, 2015

Feature request: using variables in the docker-compose files #1282

Closed

aanand mentioned this pull request Apr 30, 2015

Interpolate environment variables in docker-compose.yml #1377

Closed

relwell closed this Apr 30, 2015

jdeathe referenced this pull request in jdeathe/centos-ssh Sep 3, 2015

ISSUE 23: Use relative path to source configuration files.

82928b8

rlgomes mentioned this pull request Mar 4, 2016

enable postgres example tests juttle/juttle-engine#68

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for environment variables with default values in fig.yml #845

Add support for environment variables with default values in fig.yml #845

relwell commented Jan 15, 2015

dnephin Jan 15, 2015

relwell Jan 15, 2015

dnephin commented Jan 15, 2015

relwell commented Jan 15, 2015

thaJeztah Jan 15, 2015

aanand commented Jan 15, 2015

thaJeztah commented Jan 15, 2015

relwell commented Jan 15, 2015

thaJeztah Jan 15, 2015

relwell commented Jan 16, 2015

relwell commented Feb 6, 2015

relwell commented Feb 6, 2015

relwell commented Feb 10, 2015

thaJeztah commented Feb 10, 2015

aanand commented Feb 11, 2015

hawknewton commented Mar 4, 2015

torbjornvatn commented Apr 1, 2015

hadim commented Apr 7, 2015

relwell commented Apr 23, 2015

aanand commented Apr 30, 2015

relwell commented Apr 30, 2015

		@@ -10,6 +10,14 @@ Each service defined in `fig.yml` must specify exactly one of `image` or `build`

		As with `docker run`, options specified in the Dockerfile (e.g. `CMD`, `EXPOSE`, `VOLUME`, `ENV`) are respected by default - you don't need to specify them again in `fig.yml`.

		Fig supports using environment variables with optional backoffs as values. The accepted patterns are as follows:

Add support for environment variables with default values in fig.yml #845

Add support for environment variables with default values in fig.yml #845

Conversation

relwell commented Jan 15, 2015

dnephin Jan 15, 2015

Choose a reason for hiding this comment

relwell Jan 15, 2015

Choose a reason for hiding this comment

dnephin commented Jan 15, 2015

relwell commented Jan 15, 2015

thaJeztah Jan 15, 2015

Choose a reason for hiding this comment

aanand commented Jan 15, 2015

thaJeztah commented Jan 15, 2015

relwell commented Jan 15, 2015

thaJeztah Jan 15, 2015

Choose a reason for hiding this comment

relwell commented Jan 16, 2015

relwell commented Feb 6, 2015

relwell commented Feb 6, 2015

relwell commented Feb 10, 2015

thaJeztah commented Feb 10, 2015

aanand commented Feb 11, 2015

hawknewton commented Mar 4, 2015

torbjornvatn commented Apr 1, 2015

hadim commented Apr 7, 2015

relwell commented Apr 23, 2015

aanand commented Apr 30, 2015

relwell commented Apr 30, 2015