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

deprecate: Raise deprecation levels of API property setters #2209

Merged
merged 2 commits into from
Aug 20, 2024

Conversation

bog-walk
Copy link
Member

Description

Summary of the change: Raise the deprecation levels of property setters that were deprecated after DatabaseConfig was implemented. The deprecation level has remained untouched because they were being used for testing purposes.

Detailed description:

  • What:

The following elements have been deprecated since around 0.35.1 ->

Raise from WARNING to ERROR:

  • Database.useNestedTransactions

  • Database.defaultFetchSize

  • ThreadLocalTransactionManager.defaultIsolationLevel

  • Why: [Test refactoring]
    For the last remaining property used in tests, the only current alternative to setting the deprecated property would be to either store a configured Database.connect() instance in a variable or use TestDB.connect { setter }. Both then require using new transaction blocks and manually creating and dropping tables, which becomes tedious if multiple tables and dialects are being tested.

  • How:
    DatabaseTestsBase methods, withDb(), withTables(), and withSchemas(), now have a parameter that accepts a configuration builder, which is more easily passed to the call to connect().
    Some logic has been added to base withDb() to make sure that any database that stays registered for the entire test suite only retains the set configuration for that single call.


Type of Change

Please mark the relevant options with an "X":

  • Bump deprecation levels

Affected databases:

  • All

Checklist

  • The build is green (including the Detekt check)

Related Issues

@bog-walk bog-walk requested review from e5l and joc-a August 20, 2024 03:11
Copy link
Member

@e5l e5l left a comment

Choose a reason for hiding this comment

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

Looks good

The properties useNestedTransactions, defaultFetchSize, and defaultIsolationLevel
have deprecated setters since around version 0.35.1, since they were moved to DatabaseConfig
builder and only left in for testing.

DatabaseTestsBase methods have been refactored to accept a configuration parameter
so that withTables() for example can easily pass these properties to TestDB.connect()
as a builder.

The deprecation level of the properties has been raised to ERROR.
- Rebase from main
- Remove redundant test column qualifiers
@bog-walk bog-walk force-pushed the bog-walk/deprecate-test-only-api branch from c052c19 to 1cc1c5f Compare August 20, 2024 18:16
@bog-walk bog-walk merged commit 578d59d into main Aug 20, 2024
5 checks passed
@bog-walk bog-walk deleted the bog-walk/deprecate-test-only-api branch August 20, 2024 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants