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

Feature request: dynamic includes #122

Open
eranation opened this issue Dec 31, 2013 · 33 comments
Open

Feature request: dynamic includes #122

eranation opened this issue Dec 31, 2013 · 33 comments

Comments

@eranation
Copy link

It will be nice to be able to do this

mode = development

and somewhere else do

include ${mode}.conf

to allow different configuration files to be loaded based on other keys

@havocp
Copy link
Collaborator

havocp commented Jan 2, 2014

This would indeed be nice, but it's a little tricky in practice for a few known reasons and possibly some unknown reasons. See https://github.com/typesafehub/config/blob/master/HOCON.md#includes for spec details.

Right now the parameter to include is always a quoted string, so could be "${mode}" which should load a file that's called literally ${mode}; the ${} syntax isn't special inside quotes. So we'd have to change the parser to allow unquoted strings here.

Right now ${} resolution happens after the file has been entirely loaded, while include happens as the file is being parsed. One consequence is that ${} can normally look "forward" in the file. Either include-processing would need to be delayed, or the ${} for includes would not be able to look forward. Both could have surprising effects.

Right now files are loaded standalone as their own Config, and then reference.conf gets stuck underneath and system properties stuck on top. if ${mode} could refer to a value in reference.conf or a system property, then the include could not be processed until the Config#resolve step. But this would introduce blocking/error-causing IO into resolve(), while resolve() is purely algorithmic at the moment. i.e. we'd move loading the include from ConfigFactory#load and friends, to resolve.

Another idea in the past has been to support only system properties (not references to the config itself) in filenames to include, but that would be confusing.

For now, most people use -Dconfig.file=production.conf for this purpose. I do think it'd be a bit more elegant to put the mode setting in the config file itself (rather than by changing which config file to load), but it isn't easy to do so, so that's why we haven't.

@havocp
Copy link
Collaborator

havocp commented Jan 11, 2014

For others looking up or wondering about this question, I should mention that there are a couple of ways to support custom include syntax in an application.

One is to create a custom ConfigIncluder http://typesafehub.github.io/config/latest/api/com/typesafe/config/ConfigIncluder.html

The other is to create a custom URL protocol in the usual Java way, see http://stackoverflow.com/questions/2537255/creating-custom-uri-scheme-using-uri-class and http://www.cooljeff.co.uk/2009/12/12/custom-url-protocols-and-multiple-classloaders/ for example. Then use include url("myproto:foobar")

I'm not saying either of these is necessarily worthwhile (they are probably more work than the problem being solved), but maybe someone would find them useful in some circumstances.

@havocp
Copy link
Collaborator

havocp commented Jan 11, 2014

Another idea here could be special-case support only environment variable and/or only system property substitution using a syntax which spelled that out. Something like include file(env("$HOME/bar")) maybe. So it would be kind of opt-in. I don't know. Other ideas welcome.

@havocp
Copy link
Collaborator

havocp commented Jan 15, 2014

#134 is potentially a different solution, something like:

development {
  foo = 42
}

production {
  foo = 21
}

mode = development

foo = ${${mode}.foo}

But I've thought about it for literally 30 seconds so it might have issues.

@drdamour
Copy link

drdamour commented Feb 6, 2014

I was interested in doing something like this as well. I don't want to have a Procfile for my heroku deployments but i want the heroku server to have an enviornment variable that indicates which config file to use.

I was hoping to put include ${?ENV_CONFIG} in my play apps applicaiton.conf and have it conditionally include that file if the EVN_CONFIG env variable was present. Something like this would have gotten me there.

For now i'll use a Procfile and pass in the config via java properties

@havocp
Copy link
Collaborator

havocp commented Feb 6, 2014

on Heroku you can also set JAVA_OPTS in many cases (depending on exactly which setup you're using to launch the app). Like JAVA_OPTS='-Dconfig.file='. sbt-native-packager (new enough version) and sbt-start-script plugins support JAVA_OPTS.

@drdamour
Copy link

drdamour commented Feb 6, 2014

hmm..i tried those a bunch and never could get them to work

@havocp
Copy link
Collaborator

havocp commented Feb 6, 2014

there have been bugs in the past e.g. sbt/sbt-native-packager#47 but it depends on how you are doing things of course.

@drdamour
Copy link

drdamour commented Feb 6, 2014

not really doing anything special, heroku config:set JAVA_OPTS="-Dconfig.resource=application.qa.conf" and pushing the code, on app startup nothing in JAVA_OPTS appears to take.

@drdamour
Copy link

drdamour commented Feb 6, 2014

Yup it was java_opts vs JAVA_OPTS. somehow must be ref'ing the older version of native packager. thx

@aelsaman
Copy link

I just ran into this issue,
this works: include "fileName.conf"
But this does not work: include "${dynamicFileName}"
although I am not doing something new, just normal substitution in include statement but never works.
Hopefully this will be addressed in future releases.

@havocp
Copy link
Collaborator

havocp commented Mar 7, 2015

On #265 there was another idea -Dconfig.mergeIntoRoot, in addition to the include env() idea and nested substitution ( #134 ) idea already mentioned here.

@kirked
Copy link

kirked commented May 1, 2015

@havocp What do you think about adding special processing for this in ConfigImpl.FileNameSource.nameToParseable(), only working with environment variables (using envVariablesAsConfig() to support this feature?

To me, it only makes sense when we're talking about including files, not from classpath or URLs.

I'm thinking of something like:

        if (name.startsWith("${") && name.endsWith("}")) {
            String key = name.substring(2, name.length() - 1);
            Config env = envVariablesAsConfig();
            if (env.hasPath(key)) {
                name = env.getString(key);
            }
        }
        return Parseable.newFile(new File(name), parseOptions);

@havocp
Copy link
Collaborator

havocp commented May 2, 2015

A few issues are:

  • then you can't have a file named with ${}, which while granted is a weird filename, I do have a project checked out that contains a file called ${cache} for some reason for example
  • it's a bit confusing to have ${} that looks at env vars only, when that isn't the usual meaning in the rest of the format
  • it's a bit confusing if this only works for files and not URLs or resources

include file(env("$HOME/bar")) I like a bit more, though maybe curly braces are good there (the env maybe clarifies that it will only look at the environment).

Or another option is to follow the existing convention and require ${} to be outside of quotes (what HOCON.md calls "unquoted string"), so we'd keep all the existing literal meaning of quoted strings.

Combining those ideas, maybe include file(env(${HOME}"/bar")) or something like that? It would also support url() and classpath().

Before getting bogged down in syntax though what would be helpful I think would be to try to think through the three ideas mentioned so far - env vars in includes (with whatever syntax), -Dconfig.mergeIntoRoot, and nested substitution.

What are the new use cases that current mechanisms such as -Dconfig.file don't solve, that we want to address?
Which of the three ideas would address these new use cases and why or why not?
Any strong argument for why one of these ideas is more useful or more clear than the others?
Any ideas besides these three?

A thing I think is challenging here is that people clearly expect include ${foo} to "just work" with ${foo} having the same meaning it has elsewhere in the file. The problem is that this moves IO from the ConfigFactory.parseFile method (which you expect to throw an IO exception) to the Config.resolve method (which really should not be going out on the internet or doing any kind of IO). So I guess we just can't do exactly what people expect without causing problems.

Anyway the short version: I think the bulk of the work here is to figure out and articulate what makes sense... I hope this comment gives some idea of the considerations, but I don't know the answer yet.

Any opinions / pros-and-cons / persuasive arguments ?

Don't worry about implementation difficulty. Everything discussed so far I think isn't a big deal to code.

@fgutmann
Copy link

Our use case is to include additional config from the users home directory. Environment variables and system properties would be sufficient for us.

Something like include file(${user.home}"/.myapp.conf") would definitely help us.
I personally would prefer to not have the env() marker.

As already mentioned by others, users do expect the ${variable} syntax to just work inside of include. Now it doesn't work at all and users have to look up in the documentation why it does not work. If env() is used, the intuitive approach will still not work and everybody has to consult the docs.

In the case of the more regular syntax it will "just work" for maybe half of the users. When a referenced value inside include is not available, we should provide a nice error/warning message. This message should include the hint that only environment variables and system properties are available inside of include.

@havocp
Copy link
Collaborator

havocp commented Aug 18, 2016

I agree that something here would be nice.

As the earlier comment notes, if you try implementing to do the full thing people expect (that ${foo} can be any value in the config), I think you'll see the issue that you'd have to break the resolve() API/ABI because now there would be IO involved in that process. This is pretty difficult to deal with. In addition to throwing unexpected exceptions that method could now be arbitrarily slow when before it was reliably a CPU-only task, which I think is also an unacceptable semantic break.

Implementing it with only env vars / system properties is much easier, because it can be done file-by-file during file load, rather than during the later resolve(), but risks not being what people expect.

Anyway, something like this is probably worth pursuing.

@fgutmann
Copy link

I'm totally in favor of just implementing it with env vars / system properties because it sounds doable and will probably help in many use cases. If we agree on a syntax I could even try to implement it. But I've to warn you in advance - I have no experience with scala yet :-)

For me the risk of not being what people expect is worth being taken because:

  • It currently is not what people expect. Things will just become closer to what they expect.
  • We can tell in a warning message that only env / system properties inside include are supported
  • A new syntax is not what people expect and would make things more complicated

@havocp
Copy link
Collaborator

havocp commented Aug 18, 2016

The library is all Java, only the tests are in Scala, which is probably a good way to learn Scala in fact :-)

The hardest part is probably to allow a "string concatenation" rather than only strings inside the include(), i.e. it should be ${FOO}"/bar" not "${FOO}/bar" if that makes sense. This will require some sort of parser change, though I think the actual env/system property values can be resolved at parse time right away, so it won't be necessary to make very pervasive changes. There should also be a spec (HOCON.md) update to describe the new syntax, and some tests. I hope it is fairly straightforward.

If the parser throws an exception on undefined env/system values, then maybe that exception message could say "only env vars and system props are supported here, not values from the config itself" ?

I'm not sure what issues you'd encounter but if you give it a shot you can always ask questions as you go either on gitter or here in the issue tracker. Don't be shy about that.

@fgutmann
Copy link

Oh, i didn't recognize that it is mainly java. Just thought "typesafe ..." must be scala :-)
There are some syntax questions that are not yet clear to me.

  1. pretty clear
include ${HOME}"/"${?SUBDIR}"/file.conf"
include file(${HOME}"/"${?SUBDIR}"/file.conf")
  1. what do do with this one? According to string concatenation the whitespace inbetween should be preserved
include file(${HOME} "/" ${?SUBDIR} "/file.conf")
  1. looks doable
include ${HOME}/${?SUBDIR}/file.conf
include file(${HOME}/${?SUBDIR}/file.conf) 
  1. intresting :-)
    According to unquoted string documentation ( and ) are no forbidden chars.
    Therefore support for unquoted strings inside file(), uri() and classpath() will be quite tricky.
include file(${HOME}/${?SUBDIR}/file.conf) // would be fine
include file(${HOME}/${?SUBDIR}/fil)e.conf) // would be fine
include file(${HOME}/${?SUBDIR}/fil)e.conf // would be an error

@havocp
Copy link
Collaborator

havocp commented Aug 18, 2016

On 2), I would either preserve the whitespace or make it an error ... otherwise it's maybe not consistent. I guess preserving it is the path of least resistance.

Does it make sense to support ${?} ? In regular config files, expanding to an empty string could make sense, or having an optional override (expanding to the existing value) could make sense, but I'm not sure either one makes sense in a filename.

  1. supporting without quotes probably makes sense, I think I just required them for ease of writing the parser or something. Most likely once you support expansions you've done the work to support no-quotes. Except for your point about parens...

  2. hmm, yes. maybe we should require quotes for that reason. We could forbid those chars only in this context, but it might be simpler to say that only ${} expressions can be outside quotes. Then the parser is probably easier since you just look for quotes and $ and that's it.

@costimuraru
Copy link

Hey folks!

We have a similar use case, where we want to hit a URL to retrieve the config:

include url("https://configservice.com/"${APP}"/"${ENV}"/"${REGION}"/1.conf")

Sadly, this doesn't work :-(. Any tips?

@Yeitijem
Copy link

Hi!

I tried to make some good defaults by including a credentials file from the users directory, and failed...

I understand havocp's point. It would be nice to see the strings within include directives treated specially. Introducing env as wrapper around is acceptable but without would be nicer. Maybe corner cases could occur with file names having $ or { in there names ... if this is possible at all. Environment variables are sufficient for my problem.

Could it be a solution to add some more rules on other variables in order to allow those too without sacrificing the simplicity of the parser? (e.g. only variable definitions occured so far are accounted)

@mar-schmidt
Copy link

Seems as this feature would be really beneficial to have, voting for it to be implemented asap

@axos88
Copy link

axos88 commented Jul 10, 2019

up

@domdorn
Copy link

domdorn commented Aug 26, 2019

+1 ... also need this feature.. has anything been done in this area or is there a workaround?

@herveDarritchon
Copy link

+1 for this feature !

Meanwhile, I use -Dconfig.file=production.conf for this purpose with a common configuration file included in each specific conf file.

@fstackdev
Copy link

+1, I need it also

@joe-mojo
Copy link

Another need for this feature is :

include "file1.conf" //defines foo.bar
include "path/file_${foo.bar}.conf"

@voodoo144
Copy link

+1

@aandis
Copy link

aandis commented May 26, 2021

+1

@jroper
Copy link
Member

jroper commented Sep 8, 2021

For anyone wanting this, here's a custom file includer like @havocp talked about:

import java.io.File

import com.typesafe.config.{ ConfigIncludeContext, ConfigIncluder, ConfigIncluderFile, ConfigObject }

class ParameterizedFileConfigIncluder private (
    _fallback: ConfigIncluderFile with ConfigIncluder,
    environment: Map[String, String])
    extends ConfigIncluder
    with ConfigIncluderFile {

  def this() = this(null, sys.env)
  def this(environment: Map[String, String]) = this(null, environment)

  private val parser = "\\$\\{([^}]*)\\}".r

  private def fallback: ConfigIncluderFile with ConfigIncluder = {
    if (_fallback == null) {
      throw new IllegalStateException("No fallback has been provided")
    }
    _fallback
  }

  override def withFallback(fallback: ConfigIncluder): ConfigIncluder = {
    if (fallback eq _fallback) this
    else
      fallback match {
        case file: ConfigIncluder with ConfigIncluderFile => new ParameterizedFileConfigIncluder(file, environment)
        case _ =>
          throw new IllegalArgumentException(
            "ParameterizedFileConfigIncluder can only fallback to a ConfigIncluderFile")
      }
  }

  override def include(context: ConfigIncludeContext, what: String): ConfigObject =
    fallback.include(context, what)

  override def includeFile(context: ConfigIncludeContext, what: File): ConfigObject = {
    val substitutedFile = new File(
      parser.replaceAllIn(
        what.getPath,
        regexMatch => {
          val varName = regexMatch.group(1)
          environment.getOrElse(varName, "")
        }))
    fallback.includeFile(context, substitutedFile)
  }
}

That just reads from environment variables but could be trivially modified to read from system properties instead/as well. It can be used like so:

ConfigFactory.load(
  "application",
  ConfigParseOptions.defaults().setIncluder(new ParameterizedFileConfigIncluder()),
  ConfigResolveOptions.defaults()
)

@alfonsonishikawa
Copy link

alfonsonishikawa commented Dec 22, 2022

And what about creating a new "include eager ..."? This way you can ignore all the lazy resolutions, etc. The "eager" basically says that "it will be included with whatever values I have up to now, so for env variables and simple things it will work, but don't use messy things".

@lciolecki
Copy link

+1

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

No branches or pull requests