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

Use configuration to get 'altTablespace', 'altSchema' and 'altCatalog… #3124

Merged
merged 2 commits into from
Sep 26, 2022

Conversation

mehrabisajad
Copy link
Contributor

…' values

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

When I use the postgres integration test with an existing database (without using Docker), I get the following error.
liquibase.exception.DatabaseException: ERROR: tablespace "liquibase2" does not exist

The "liquibase2" value was hardcoded. These constant values are removed from the code to be read from the configuration file.

@nvoxland nvoxland added the SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions label Aug 30, 2022
Copy link
Contributor

@nvoxland nvoxland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review and test results:

Things to be aware of:

  • Changes how we get schema/catalog settings in our integration tests
  • Does not impact production code

Things to worry about:

  • Nothing

@github-actions
Copy link

github-actions bot commented Aug 30, 2022

Unit Test Results

  4 620 files  ±0    4 620 suites  ±0   37m 48s ⏱️ - 1m 38s
  4 617 tests ±0    4 398 ✔️ ±0     219 💤 ±0  0 ±0 
54 576 runs  ±0  49 552 ✔️ ±0  5 024 💤 ±0  0 ±0 

Results for commit ef7297f. ± Comparison against base commit 369ba21.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@XDelphiGrl XDelphiGrl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR enhances tests to validate alt* properties are cleared to ensure proper cleanup of cases where Liquibase replaces schemaName with catalogName.

  • See line 200 of liquibase-integration-tests/src/test/java/liquibase/dbtest/AbstractIntegrationTest.java for the full comment on these cases.
  • PR also enhances test to validate the altTablespace property is applied when creating the Liquibase tracking tables.
  • No additional testing required.

APPROVED

@nvoxland nvoxland merged commit 7671132 into liquibase:master Sep 26, 2022
@nvoxland nvoxland added this to the 1NEXT milestone Sep 30, 2022
@tabbyf00
Copy link

Thanks for your PR submission! We just finished reviewing and merging it into the 4.17.0 release on October 10, 2022. When you get a chance, could you please Star the Liquibase project? The star button is in the upper right corner of the screen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autocandidate complexityLocal criticalityBlocker SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions Severity3 sprint2022-34 TypeBug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

6 participants