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

feat: EXPOSED-555 Allow read-only suspendable transactions #2274

Merged
merged 3 commits into from
Nov 27, 2024

Conversation

RenanKummer
Copy link
Contributor

@RenanKummer RenanKummer commented Oct 14, 2024

Description

Summary of the change: Regular transaction management allow callers to configure if new transactions should be read-only. This change adds the same behavior to suspendable transactions.

Detailed description:

  • What: A readOnly parameter has been added for newSuspendTransaction.
  • Why: Regular transactions allow readOnly to be configured by callers. As per JetBrains Exposed documentation, this behavior is supposed to be the same for regular and suspendable transactions, but the parameter was missing for suspendable transactions.
  • How: The parameter was added as optional (nullable with the default value set to null) to avoid breaking changes as much as possible. It is passed ahead to internal functions in order to create the transaction either as read-only based on caller's configuration or as the original behavior for existing code.

Type of Change

Please mark the relevant options with an "X":

  • New feature

Updates/remove existing public API methods:

  • Is breaking change

Affected databases:

  • Mysql5
  • Mysql8
  • Postgres

Checklist

  • Unit tests are in place
  • The build is green (including the Detekt check)
  • All public methods affected by my PR has up to date API docs
  • Documentation for my change is up to date

Related Issues

https://youtrack.jetbrains.com/issue/EXPOSED-555/newSuspendedTransaction-support-for-readOnly
EXPOSED-243

@RenanKummer RenanKummer changed the title feat(EXPOSED-555): Allow read-only suspendable transactions feat!(EXPOSED-555): Allow read-only suspendable transactions Oct 14, 2024
@RenanKummer RenanKummer changed the title feat!(EXPOSED-555): Allow read-only suspendable transactions feat!: EXPOSED-555 Allow read-only suspendable transactions Oct 14, 2024
@bog-walk bog-walk self-requested a review November 26, 2024 16:22
@bog-walk bog-walk self-assigned this Nov 26, 2024
Copy link
Member

@bog-walk bog-walk left a comment

Choose a reason for hiding this comment

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

Thanks for working on this feature request @RenanKummer
Please consider rebasing from main and addressing changes below.

This PR has been labelled feat! but the BreakingChanges docs has not been updated. Given the default argument, the likeliness of using a statement lambda, and our versioning, it would be fine to label feat.
But if you're concerned about non-named arguments in the parameter list and wish to keep this label for thoroughness, then please also add a note under section 0.57.0 of the BreakingChanges doc.

Regular transaction management allow callers to configure if new transactions should be read-only. This change adds the same behavior to suspendable transactions.
@RenanKummer RenanKummer changed the title feat!: EXPOSED-555 Allow read-only suspendable transactions feat: EXPOSED-555 Allow read-only suspendable transactions Nov 26, 2024
@RenanKummer
Copy link
Contributor Author

@bog-walk - Could you please review again? I applied the suggested changes.

@e5l e5l merged commit 7e5d03f into JetBrains:main Nov 27, 2024
3 checks passed
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.

3 participants