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

fix: commit regardless of framework's transaction_writes count #25322

Merged
merged 1 commit into from Mar 11, 2024

Conversation

ankush
Copy link
Member

@ankush ankush commented Mar 11, 2024

  • There is code that depends on "commit", everything that happens with db.after_commit.
  • There are operations that will not write anything to DB but just
    enqueue the function, if it's enqueue_after_commit then it will break.
  • Empty commit/rollback both are basically free. MySQL treats transaction as read-only until a write is made.

- There is code that depends on "commit", everything that happens with `db.after_commit`.
- There are operations that will not write anything to DB but just
  enqueue the function, if it's enqueue_after_commit then it will break.
@ankush ankush requested a review from a team as a code owner March 11, 2024 09:09
@ankush ankush requested review from akhilnarang and removed request for a team March 11, 2024 09:09
@ankush ankush added backport version-14-hotfix backport to version 14 backport version-15-hotfix Backport the PR to v15 labels Mar 11, 2024
@github-actions github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Mar 11, 2024
@ankush ankush marked this pull request as draft March 11, 2024 09:28
@ankush
Copy link
Member Author

ankush commented Mar 11, 2024

This can break code like this:

@frappe.whitelist()
def enqueue_job():
    try:
        frappe.enqueue("job", enqueue_after_commit=True)
        1/0 # Any exception really
    except Exception:
        pass

Previously this job was not queued because we didn't see any writes hence no commit.

@ankush ankush marked this pull request as ready for review March 11, 2024 09:38
@ankush ankush removed the add-test-cases Add test case to validate fix or enhancement label Mar 11, 2024
@ankush ankush merged commit acf398f into frappe:develop Mar 11, 2024
24 checks passed
@ankush ankush deleted the commit_transaction_writes branch March 11, 2024 09:48
mergify bot pushed a commit that referenced this pull request Mar 11, 2024
- There is code that depends on "commit", everything that happens with `db.after_commit`.
- There are operations that will not write anything to DB but just
  enqueue the function, if it's enqueue_after_commit then it will break.

(cherry picked from commit acf398f)
mergify bot pushed a commit that referenced this pull request Mar 11, 2024
- There is code that depends on "commit", everything that happens with `db.after_commit`.
- There are operations that will not write anything to DB but just
  enqueue the function, if it's enqueue_after_commit then it will break.

(cherry picked from commit acf398f)
ankush added a commit that referenced this pull request Mar 11, 2024
… (#25328)

- There is code that depends on "commit", everything that happens with `db.after_commit`.
- There are operations that will not write anything to DB but just
  enqueue the function, if it's enqueue_after_commit then it will break.

(cherry picked from commit acf398f)

Co-authored-by: Ankush Menat <ankush@frappe.io>
ankush added a commit that referenced this pull request Mar 11, 2024
… (#25327)

- There is code that depends on "commit", everything that happens with `db.after_commit`.
- There are operations that will not write anything to DB but just
  enqueue the function, if it's enqueue_after_commit then it will break.

(cherry picked from commit acf398f)

Co-authored-by: Ankush Menat <ankush@frappe.io>
frappe-pr-bot pushed a commit that referenced this pull request Mar 12, 2024
## [14.67.1](v14.67.0...v14.67.1) (2024-03-12)

### Bug Fixes

* Add `is_virtual` in bootstrap schema ([cbc4640](cbc4640))
* **auto_email_report:** check if column (docfield) parent is a child table ([#25335](#25335)) ([#25341](#25341)) ([3f0c823](3f0c823))
* commit regardless of framework's transaction_writes count ([#25322](#25322)) ([#25327](#25327)) ([0ee8e30](0ee8e30))
* delete doctype cache before starting migration ([2c1e615](2c1e615))
* Delete EPS logs before deleting user (backport [#25248](#25248)) ([#25249](#25249)) ([0dee65c](0dee65c))
* do not mark custom field as translatable by default (backport [#25285](#25285)) ([#25329](#25329)) ([c46b7ff](c46b7ff))
* explicit optional ([71af71a](71af71a))
* ignore route history and access log from link check ([5344c8e](5344c8e))
* mac shortcuts ([ec9164f](ec9164f))
* missing timeline timestamp for comments ([294a0a3](294a0a3)), closes [/github.com//pull/24884/files#r1519195726](https://github.com//github.com/frappe/frappe/pull/24884/files/issues/r1519195726)
* **ModuleDef:** Add list view filter options to app_name Select ([253e9a0](253e9a0))
* Private images in PDFs from background jobs ([#24980](#24980)) ([7d4eaa6](7d4eaa6))
* rollback invalid customize form changes (backport [#25198](#25198)) ([#25339](#25339)) ([e833e09](e833e09))
* show "Shift" symbol istead of text ([41759ff](41759ff))
* Skip virtual doctypes in link field checks ([#24759](#24759)) ([#25332](#25332)) ([3fe2372](3fe2372))
* **uninstall:** don't allow uninstalling an app if we have other apps dependent on it ([8d04076](8d04076))
frappe-pr-bot pushed a commit that referenced this pull request Mar 12, 2024
## [15.17.2](v15.17.1...v15.17.2) (2024-03-12)

### Bug Fixes

* Add `is_virtual` in bootstrap schema ([cf8078d](cf8078d))
* **auto_email_report:** check if column (docfield) parent is a child table ([#25335](#25335)) ([#25342](#25342)) ([a870b5a](a870b5a))
* commit before ddl ([092bd0e](092bd0e))
* commit regardless of framework's transaction_writes count ([#25322](#25322)) ([#25328](#25328)) ([c7c726e](c7c726e))
* delete doctype cache before starting migration ([74a8d87](74a8d87))
* Delete EPS logs before deleting user ([1f47f7a](1f47f7a))
* do not mark custom field as translatable by default ([#25330](#25330)) ([841d885](841d885))
* dont set fetch from if partial ([800298d](800298d))
* formatting ([7f147b1](7f147b1))
* ignore route history and access log from link check ([5061a08](5061a08))
* list view click and drag select rows ([24d713e](24d713e))
* loosen validation on fetch from columns ([#25247](#25247)) ([0c934de](0c934de))
* MacOS shortcuts (backport [#25272](#25272)) ([#25280](#25280)) ([785b5c2](785b5c2))
* missing timeline timestamp for comments ([27357fb](27357fb)), closes [/github.com//pull/24884/files#r1519195726](https://github.com//github.com/frappe/frappe/pull/24884/files/issues/r1519195726)
* **ModuleDef:** Add list view filter options to app_name Select ([7357c5e](7357c5e))
* **patch:** Remove obviously invalid fetch from expressions (backport [#25284](#25284)) ([#25302](#25302)) ([0ec4bc8](0ec4bc8))
* rollback invalid customize form changes ([#25198](#25198)) ([d6bff86](d6bff86))
* Setup fail if fail function exists ([4d5e453](4d5e453))
* Skip virtual doctypes in link field checks ([#24759](#24759)) ([#25333](#25333)) ([45499c2](45499c2))
* **uninstall:** don't allow uninstalling an app if we have other apps dependent on it ([b631112](b631112))
* Use debug log to log DB queries (backport [#25265](#25265)) ([#25301](#25301)) ([24479ca](24479ca))
* **UX:** Nudge to disable user instead of deleting ([1da957f](1da957f))

### Performance Improvements

* Add optional socketio native packages ([160cf29](160cf29))

### Reverts

* fetch validations (backport [#25343](#25343)) ([#25344](#25344)) ([f729817](f729817))
* fetch validations (backport [#25343](#25343)) ([#25345](#25345)) ([7cc3686](7cc3686))
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport version-14-hotfix backport to version 14 backport version-15-hotfix Backport the PR to v15
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant