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

Allow injecting a complete Configuration #1754

Closed
pauxus opened this issue Aug 24, 2017 · 1 comment
Closed

Allow injecting a complete Configuration #1754

pauxus opened this issue Aug 24, 2017 · 1 comment

Comments

@pauxus
Copy link
Contributor

@pauxus pauxus commented Aug 24, 2017

Since we have the FlywayConfiguration it might be useful to be able to pass an implementation of that interface to Flyway to complete it as a whole (Flyway.copyConfigurationFrom(FlywayConfiguration) or Flyway.setConfiguration(FlywayConfiguration)).

This would allow to directly set the configuration from different models like the Maven Plugin, a Gradle extension or a DevOps Model, effectively reducing effort and risk when introducing new options.

In our specific use case, we have most Flyway specific configuration (like JDBC url and credentials) in a Klum based Model anyway. Currently, we convert the model manually into a flyway configuration, but this seems rather unnecessary. Most of the Flyway code uses the Configuration object already.

There are IMHO two possible approaches to this:

  • Quick and (somewhat) dirty: move the copy statements into the the new copyFromConfiguration() method
  • refactor the actual configuration data in Flyway into a new class DefaultFlywayConfiguration and delegate all access to confiuration data in Flyway to that new instance. Flyway could then pass that instance instead of this to all subsystems

Both approaches should be completely backward compatible.

I think the second approach is nicer from a SOC point of view, as it allows all configuration related code to get its own place.

I can create a pull request for that.

@axelfontaine axelfontaine added this to the Flyway 5.0.0 milestone Nov 25, 2017
axelfontaine added a commit to flyway/flywaydb.org that referenced this issue Nov 25, 2017
@axelfontaine
Copy link
Contributor

@axelfontaine axelfontaine commented Nov 25, 2017

There is now a Flyway(FlywayConfiguration) constructor.

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

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.