Update Ant Tasks to use DatabaseFactory rather than create connection itself #1427
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #1427 +/- ##
============================================
+ Coverage 47.51% 48.25% +0.73%
+ Complexity 8053 7915 -138
============================================
Files 801 793 -8
Lines 39753 38454 -1299
Branches 7275 6894 -381
============================================
- Hits 18890 18557 -333
+ Misses 18353 17445 -908
+ Partials 2510 2452 -58
Continue to review full report at Codecov.
|
|
Thanks @mattbertolini We take a look at this and get it into a release soon. |
molivasdat
added
DBAll
ImpactHigh
IntegrationAnt
RiskLow
This is not an exhaustive list of tests but it's better than nothing and a decent start. As the class is refactored tests can be improved.
…river properties.
mattbertolini
force-pushed
the
mrb/ant-tasks-to-database-factory
branch
from
93305c9
to
532b213
Compare
nvoxland
approved these changes
Nov 3, 2020
|
Cherry picked the changes into 4.2.x |
This was referenced Nov 16, 2020
This was referenced Dec 7, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
name: Pull Request
about: Create a report to help us improve
title: ''
labels: Status:Discovery
assignees: ''
Environment
Liquibase Version: 3.10.x
Liquibase Integration & Version: Ant
Liquibase Extension(s) & Version: N/A
Database Vendor & Version: N/A
Operating System Type & Version: All
Pull Request Type
Fixes #1378
Description
I have been investigating #1378 and I believe the issue is the Ant tasks are creating the
DatabaseConnectionthemselves and not respecting the logic found in theDatabaseFactory. This PR updates the Ant tasks to leverage theDatabaseFactoryto keep consistent with the rest of the integration types (CLI, Maven, etc).This involved a small refactoring of the
DatabaseFactoryso it can accept a pre-configuredPropertiesobject as that is needed by Ant. The refactoring is entirely backward compatible and was accomplished using simple method and argument extraction. I added several unit tests before the refactor to make sure compatibility was maintained.This would be good to backport into 3.10.x but I will leave that up to you.
Steps To Reproduce
The bug is a bit difficult to reproduce. See #1378 for details. I believe there is a small race condition that can cause the exception where a pro connection is used in the tasks when its not desired. The changes in this PR should remove the race condition.
Actual Behavior
A clear and concise description of what happens in the software before this pull request.
Expected/Desired Behavior
A clear and concise description of what happens in the software after this pull request.
Screenshots (if appropriate)
If applicable, add screenshots to help explain your problem.
Additional Context
Add any other context about the problem here.
Fast Track PR Acceptance Checklist:
Need Help?
Come chat with us on our discord channel