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

Rewrite Maven Integration to use Command Framework #3285

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nvoxland
Copy link
Contributor

Impact

  • Bug fix (non-breaking change which fixes expected existing functionality)
  • Enhancement/New feature (adds functionality without impacting existing logic)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

We introduced the command framework to be a consistent business logic layer across all the integrations. Same command names, same arguments for commands, same extensibility pipeline, etc. With the command framework, each integration should just be a thin layer translating the user request that is specific to that integration to the generic command calls.

We rewrote the CLI to use the command framework a while ago, this applies the same pattern to Maven.

Things to be aware of

  • See discussion

Things to worry about

  • Need to preserve existing maven goals/arguments for backwards compatibility

@github-actions
Copy link

Unit Test Results

0 files   -   4 620  0 suites   - 4 620   0s ⏱️ - 34m 31s
0 tests  -   4 620  0 ✔️  -   4 401  0 💤  -    219  0 ±0 
0 runs   - 54 612  0 ✔️  - 49 588  0 💤  - 5 024  0 ±0 

Results for commit e19b281. ± Comparison against base commit c3b320c.

@nvoxland
Copy link
Contributor Author

Marked this as a draft PR because it's very much a work in progress still and currently focused around "can it work" vs. "making it work".

My biggest question to answer first was "Can maven support the dynamic goals/parameters/classes needed for our plugin-based command framework?"

What I found so far is:

  1. I can create a single class which can dynamically take any goal and configuration
  2. Maven requires a plugin.xml file in the jar which statically lists the available goals and possible configuration settings for those goals.

The new LiquibaseMojo class handles 1 above.

For 2, I so far have liquibase/liquibase-sdk-maven-plugin#7 which will generate a plugin.xml file based on the command framework. I'm not super-happy about that setup but it works for now:

  • I found I had to have the logic in a separate repo's maven goals, using a maven plugin from another sub-module in the same project wasn't working well
  • The maven code that gets the plugin descriptor from the plugin.xml is pluggable/configurable so in theory we should be able to have a separate class for that. But as far as I can tell so far, that configuration is global to the maven install and not configurable within the plugin so not something we can override. Also, I'd guess there are external tools (like IDEs) that assume a plugin.xml and doing something fancy will confuse those. So seem stuck with a plugin.xml?

Assuming we need that static plugin.xml, we still need a way to handle the fact that we have extensions that add additional commands, global settings, and command arguments.

All I can think of so far is having a more generic/unmanaged map of arguments in a single goal. Like calling mvn liquibase:run with a configuration of:

    <configuration>
                    <command>update</command>
                    <commandArguments>
                        <changelogFile>/liquibase-testing/liquibase-0-SNAPSHOT</changelogFile>
                        ...
                    </commandArguments>
                    <globalSettings>
                        <searchPath>...</searchPath>
                        ...
                    </globalSettings>
                </configuration>

where commandArguments and globalSettings are just mapped in Maven to Maps and can be anything. Like how the Ant maven support works.

But while that lets us do what we want, it's not user friendly because you loose all the goal help and intellisense.

So I'm thinking we want to support both: have a generic liquibase:run call like that, but also have plugin.xml with all the built-into-core commands and arguments available for the majority of people who just do that.

And even for people using those pre-made goal configurations, I found we can get all the passed user parameters even if they aren't mapped to specific configurations, so with our ValueProvider support any extension command arguments and global settings can still be set via maven properties.

What do people think on that compromise?

@nvoxland
Copy link
Contributor Author

For my second experiment, I created #3286 which will likely eventually merge into here. But keeping them separate discussions for now. This is just on the "generic maven wrapper" and the other is on the "generic integration code"

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

Successfully merging this pull request may close these issues.

None yet

2 participants