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

Support multiple tasks of the same type in gradle #738

Closed
jnizet opened this issue Apr 25, 2014 · 13 comments
Closed

Support multiple tasks of the same type in gradle #738

jnizet opened this issue Apr 25, 2014 · 13 comments

Comments

@jnizet
Copy link

@jnizet jnizet commented Apr 25, 2014

I have a unit-test database, and an integration-test database (and I could have more for production, for example), each with a different user/password/url. And I'd like to be able to create flyway tasks for each of them:

class PostgresDatabase {
    String name;
    String userName;
    String password;
    String url;

    PostgresDatabase(String name, String userName, String password, String url) {
        this.name = name;
        this.userName = userName
        this.password = password;
        this.url = url;
    }
}

def unit = new PostgresDatabase('unit', 'foo_unit', 'foo_unit', 'jdbc:postgresql://localhost/foo_unit');
def integ = new PostgresDatabase('integ', 'foo_integ', 'foo_integ', 'jdbc:postgresql://localhost/foo_integ');

[integDatabase, unitDatabase].each {db ->
    project.task('flywayMigrate' + db.name.capitalize(), type: org.flywaydb.gradle.task.FlywayMigrateTask) {
        ...
    }
}

Unfortunately, that's not as easy as it could be, because the Flyway gradle tasks don't have any property. They all read their properties from a unique flyway extension.

It would be nice to be able to create tasks having unique properties, overriding those from the extension.

It would also be nice to be able to use a flyway-base-plugin, which would only make the tasks available (for my use-case, for example), and a flyway-plugin, relying on the base plugin, and which would add default tasks using the extension.

I've managed to do it by subclassing the Flyway tasks and overriding the createFlyway() method, but that's painful, and relies on an undocumented implementation detail.

@axelfontaine
Copy link
Contributor

@axelfontaine axelfontaine commented Apr 26, 2014

Thanks for the feature request. I can definitely see the need for this. The goal should be for a syntax the keeps the simple case simple (current situation) while enabling more advanced scenarios (your feature request).

Which one of the syntax variants presented here http://gradle.1045684.n5.nabble.com/Simulating-Maven-Profiles-td3374782.html would you favor and why? Or something else completely?

@jnizet
Copy link
Author

@jnizet jnizet commented Apr 26, 2014

Actually, I realize that the documentation of the gradle plugin at https://github.com/flyway/flyway/tree/master/flyway-gradle-plugin is different from the documentation at http://flywaydb.org/documentation/gradle/.

In the Github project README, which doesn't seem to reflect the reality of the implementation of the plugin, the flyway extension allows defining defaults, and specific configurations for several databases. This looks like what I want. Except it would be nice if, in addition to the flyway tasks which apply on all the defined databases, there was one task for each database:

  • flywayMigrateTransactional would migrate the "transactional" database
  • flywayMigrateReporting would migrate the "reporting" database
  • flywayMigrate would migrate all the defined databases
  • same for the other tasks (flywayCleanTransactional, flywayCleanReporting, flywayClean for example).

Another option would be to provide a flyway-base plugin which would only provide configurable Flyway tasks. It would be up to the user to add any of those tasks to its project and to configure them as he wants (as in http://gradle.1045684.n5.nabble.com/Simulating-Maven-Profiles-td3374782.html#a3375218, where several jar tasks are added to the project and configured independantly). The flyway plugin would add the flyway extension to the project, and would add the flyway tasks as it does now, configuring the tasks using the extension.

Note that the two approaches can be combined, and that would be the best of both idea:

  • flyway-base providing the configurable tasks only, for maximum flexibility
  • flyway plugin providing the multi-database extension, and defining a set of flyway tasks for every database defined in the extension, as well as a set of tasks applying to all databases.

@axelfontaine
Copy link
Contributor

@axelfontaine axelfontaine commented Apr 26, 2014

Thanks for pointing it out and sorry about the confusion. I just zapped that old file.

I would like to stick to a single plugin to avoid confusion.

Don't hesitate to post some examples of what this would look like in a build script. This will make it easier to visualize and decide.

@jnizet
Copy link
Author

@jnizet jnizet commented Apr 26, 2014

Having 2 plugins is a common pattern in Gradle. For example, java is based on the java-base plugin. cargo is based on the cargo-base plugin. That doesn't mean there should be 2 jars. Just one jar, but the end-user is free to apply the plugin he wants to. That said, now that I think about it, if the base plugin doesn't do anything, it's unnecessary. You just need to have the plugin jar file in the classpath to be able to use its tasks.

The example described in the github README looks fine to me: default properties applying to all the database, and the possibility to add or override properties in one or several database-specific sections.

Regarding my idea to have configurable tasks, the configuration would look the same as in the extension object. For example, to define a migrateUnitDb task, we would use

task migrateUnitDb(type: org.flywaydb.gradle.task.FlywayMigrateTask) {
    url  = 'jdbc://postgresql/localhost/unit'
    user = 'test'
    password = 'mySecretPwd'
    placeholders = [
        'keyABC': 'valueXYZ'
        'otherplaceholder': 'value123'
    ]
}

and to define a migrateIntegDb task, we would use

task migrateIntegDb(type: org.flywaydb.gradle.task.FlywayMigrateTask) {
    url  = 'jdbc://postgresql/localhost/integ'
    user = 'integ'
    password = 'mySecretPwd'
    placeholders = [
        'keyABC': 'valueXYZ'
        'otherplaceholder': 'value123'
    ]
}

This allows for maximum flexibility: I can add new tasks in a loop, as shown in my initial feature request, for example.

@axelfontaine
Copy link
Contributor

@axelfontaine axelfontaine commented Apr 26, 2014

Hmmm what doesn't convince me about that approach (this is also the one from the jar task in the mailing list link) is that if you now want to invoke info or clean you still have to redefine everything...

@jnizet
Copy link
Author

@jnizet jnizet commented Apr 26, 2014

Defining tasks explicitely would be advanced usage, for maximum flexibility. Nothing forbids me to loop over a set of Database objects to define tasks dynamically for each of them, and to apply a common set of properties to every task of type org.flywaydb.gradle.task.AbstractFlywayTask. For example:

// common properties applying to all databases and all tasks:
tasks.withType(org.flywaydb.gradle.task.AbstractFlywayTask) {
    password = 'mySecretPwd'
    placeholders = [
        'keyABC': 'valueXYZ'
        'otherplaceholder': 'value123'
    ]
}

That would apply the common set of properties to all the flyway tasks in my project, without duplication. That's the beauty of gradle: it provides a rich API, which doesn't limit the developer to a static, purely declarative approach.

But as I said, for simple usage, used by default by 99% of the users, what you had in the github README looked like a good idea: you define default and specific properties in a single flyway extension, containing 1 to N database definitions, and the plugin defines a set of tasks for every declared database, and a set of tasks for all databases.

@netmikey
Copy link

@netmikey netmikey commented Feb 9, 2015

I don't think there's a need for 2 separate plugins. As Peter Niederwieser (a Gradle core dev) points out in this SO question http://stackoverflow.com/a/18142219/737587

Basically the task class(es) need to declare all properties (or more) that the extension has, using the extension properties as defaults. Have a look at the code quality plugins in the Gradle codebase (e.g. CheckstylePlugin or FindbugsPlugin). By convention, concrete task classes don't have Task in their name.

Implementing this would solve all your problems: it would make the current syntax continue working without any change, and it would allow ppl to create new flyway tasks and configure them ad-hoc, effectively overriding the defaults from the extension.

@ben-manes
Copy link
Contributor

@ben-manes ben-manes commented Mar 1, 2015

If this is reintroduced, the pull request where this feature was originally implemented may be a good reference for whomever resolves this issue.

@alkamo
Copy link

@alkamo alkamo commented Feb 10, 2016

The issue that tends to make this require a second "plugin" is the configuration. Any way I can envision changing the configuration to support multiple databases would tend to break backwards-compatibility.

I'd love to tackle this problem again, but I don't have any good ideas about how to manage the configuration while satisfying both needs: 1) support the simplest configuration and 2) only implement a single plugin.

@alkamo
Copy link

@alkamo alkamo commented Feb 11, 2016

I have a potential solution for this: maintain the existing extension properties and add a collection of extension objects within it as well. If the collection is empty, the plugin continues to function exactly as it does today. Otherwise, it treats the top-level properties as defaults which get overwritten by the specifics in the collection (if they overlap).

This is very similar to what I implemented before, except that it allows the current functionality to persist.

The one problem I've run into with this solution is that I can't figure out how to add containers to the plugin in Java. @axelfontaine, how do you feel about including a dummy java class to fulfill the JavaDoc requirement instead of requiring FlywayPlugin to be Java?

@alkamo
Copy link

@alkamo alkamo commented Apr 29, 2016

I now have a functional solution for this issue, using Gradle rules to apply tasks to individual configurations. I am holding off on the pull request for the full solution as the precursor hasn't been merged yet.

@gregopet
Copy link

@gregopet gregopet commented Nov 4, 2016

I've just come across the same issue with my prod & staging database: any chance of these pull requests getting merged? My current workaround is to scan project.gradle.startParameter.taskNames to see which settings to apply in the configuration closure though this obviously prevents me from migrating both databases with a single invocation.

axelfontaine added a commit to flyway/flywaydb.org that referenced this issue Feb 7, 2017
pradheeps pushed a commit to pradheeps/flyway that referenced this issue Mar 7, 2017
@cdalexndr
Copy link

@cdalexndr cdalexndr commented Nov 10, 2017

An example or an update to your site documentation would be helpful.

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
7 participants