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.

Sign up for GitHub

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

Jump to bottom

support concurrent build in maven plugin #4461

Merged
merged 1 commit into from Aug 29, 2023
Merged

support concurrent build in maven plugin #4461

merged 1 commit into from Aug 29, 2023

Conversation

piotrwielgolaski-tomtom
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

This is remaining part of #3446

To make liquid base work with maven concurrent build (-T 1C)

It uses scope manager, but it need to be set in static so before any plugins action kick in.
Also it should not inherit any scope, so each plugin execution can start on clean scope isolated from another.

@filipelautert filipelautert added TypeEnhancement SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions and removed SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions labels Jul 3, 2023
@piotrwielgolaski-tomtom
Copy link
Contributor Author

@filipelautert I noticed issue with test, fixed it now

@filipelautert filipelautert added SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions and removed SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions labels Jul 4, 2023
@piotrwielgolaski-tomtom
Copy link
Contributor Author

@filipelautert any chance to get this under review?

@trico-tillster
Copy link

@piotrwielgolaski-tomtom I still see this issue with latest version 4.23 using -T2. Will your PR fix this? Thank you
Error: Failed to execute goal org.liquibase:liquibase-maven-plugin:4.23.0:update (liquibase) on project ordering-db-schema: Cannot end scope ztupxdjtqp when currently at scope root

@piotrwielgolaski-tomtom
Copy link
Contributor Author

piotrwielgolaski-tomtom commented Jul 17, 2023 •
edited

@piotrwielgolaski-tomtom I still see this issue with latest version 4.23 using -T2. Will your PR fix this? Thank you

Based on my testing, yes it should, but keep in mind that I focused on offline database use case.
anyway for sure it will bring us closer for all cases.

@filipelautert filipelautert self-assigned this Jul 21, 2023
@trico-tillster
Copy link

@piotrwielgolaski-tomtom I've patched liquibase with this fix and I can confirm that the scope issue regarding maven concurrency that I was observing in 4.23.0 is fixed. Thank you
@filipelautert Thank you for picking this up and move it forward.

@filipelautert filipelautert force-pushed the feature/maven_concurrent_build branch from a058560 to 82c88f4 Compare July 24, 2023 14:22
@filipelautert filipelautert added SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions and removed SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions labels Jul 24, 2023
@filipelautert filipelautert mentioned this pull request Jul 24, 2023
Open
2 tasks
@piotrwielgolaski-tomtom
Copy link
Contributor Author

@rberezen it looks it is waiting for your approval, please find time to take a look at.

@rberezen rberezen added SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions and removed SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions labels Aug 5, 2023
@filipelautert
Copy link
Collaborator

Hello @piotrwielgolaski-tomtom - our build process is going through some refactoring and it is blocking @rberezen . Once it is done/fixed he will be able to finish the review here... Thanks for your patience!

@rberezen
Copy link
Contributor

rberezen commented Aug 7, 2023

@piotrwielgolaski-tomtom Apologies for the inconvenience. I will approve the request as soon as we complete the build process refactoring.

@rberezen rberezen removed the SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions label Aug 8, 2023
@rberezen rberezen added the SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions label Aug 17, 2023
@rberezen rberezen added SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions and removed SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions labels Aug 17, 2023
@filipelautert filipelautert added SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions and removed SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions labels Aug 29, 2023
@filipelautert filipelautert merged commit 0079b9c into liquibase:master Aug 29, 2023
26 of 35 checks passed
@filipelautert filipelautert added this to the 1NEXT milestone Aug 29, 2023
@piotrwielgolaski-tomtom piotrwielgolaski-tomtom deleted the feature/maven_concurrent_build branch October 4, 2023 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@filipelautert filipelautert filipelautert approved these changes

@rberezen rberezen rberezen approved these changes

@suryaaki2 suryaaki2 suryaaki2 approved these changes

Assignees

@filipelautert filipelautert

Labels
SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions sprint2023-56 TypeEnhancement
Projects
Liquibase Open Source
Archived in project
Milestone
v4.24.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@piotrwielgolaski-tomtom @trico-tillster @filipelautert @rberezen @suryaaki2