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

configure evolutions base directory with evolutions.basePath #3967

Conversation

stephennancekivell
Copy link

Solution to this issue. #3966

@@ -79,10 +80,13 @@ case class DownScript(evolution: Evolution) extends Script {
*/
object Evolutions {

val conf = ConfigFactory.load();
val evolutionsBasePath = conf.getString("evolutions.basePath", "evolutions")
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't be reparsing the entire configuration statically just to get this configuration - the base path should be taken from an injection configuration object.

@jroper
Copy link
Member

jroper commented Feb 25, 2015

I think the best thing to do here is to remove these methods from the Evolutions object, they aren't helpful there, and rather just do the string building directly in EnvironmentEvolutionsReader, ClassLoaderEvolutionsReader etc. The EnvironmentEvolutionsReader is what gets used in Play, it can also read the configuration from an injected configuration.

@jroper
Copy link
Member

jroper commented Feb 25, 2015

Actually, EnvironmentEvolutionsReader should depend on EvolutionsConfig, and this configuration should be added there, as you suggest in the issue.

As for configuration item names etc, it should be configurable globally and per database, and be something like this:

  • The directory can be configured per datasource using play.modules.evolutions.db.<datasourcename>.path. So, if play.modules.evolutions.db.default.path = "foo", then evolutions are read from foo/1.sql etc.
  • If it's not configured at the datasource level, that it defaults to ${play.modules.evolutions.path}/<datasourcename>. So, if play.modules.evolutions.path = "bar", then evolutions for default are read from bar/default/1.sql.
  • The default for play.modules.evolutions.path is evolutions.

I also think we should stop reading from the filesystem, just read straight from the classpath.

@stephennancekivell
Copy link
Author

Thanks James, Ill look into it.

On Wed, Feb 25, 2015 at 12:25 PM, James Roper notifications@github.com
wrote:

Actually, EnvironmentEvolutionsReader should depend on EvolutionsConfig,
and this configuration should be added there, as you suggest in the issue.

As for configuration item names etc, it should be configurable globally
and per database, and be something like this:

  • The directory can be configured per datasource using
    play.modules.evolutions.db..path. So, if play.modules.evolutions.db.default.path
    = "foo", then evolutions are read from foo/1.sql etc.
  • If it's not configured at the datasource level, that it defaults to
    ${play.modules.evolutions.path}/. So, if play.modules.evolutions.path
    = "bar", then evolutions for default are read from bar/default/1.sql.
  • The default for play.modules.evolutions.path is evolutions.

I also think we should stop reading from the filesystem, just read
straight from the classpath.


Reply to this email directly or view it on GitHub
#3967 (comment)
.

@stephennancekivell
Copy link
Author

Im closing this issue, Im not going to finish it. We're using symlinks instead.

I did look into doing as you suggested James, but the rabbit hole was too deep.

@mkurz
Copy link
Member

mkurz commented Aug 14, 2023

This has finally been implement in #11917

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.

3 participants