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: lstrip for query writes detection #26180

Merged

Conversation

Don-Leopardo
Copy link
Contributor

@Don-Leopardo Don-Leopardo commented Apr 27, 2024

Please provide enough information so that others can review your pull request:

There was a problem with the write detection for frappe.db.sql
This PR should be backported to v14 and v15

Explain the details for making this change. What existing problem does the pull request solve?

This PR comes to solve a problem where a write operation is not detected because of the way it is formatted. A common way to formate a query is the following:

UPDATE_QUERY = """
    UPDATE `tabMyDoctype`
    SET my_field = "My Value"
    WHERE name = %(name)s
"""

And this method of detecting writes was not working (and leaded us to a 2hs debug session of why the endpoint was not doing a commit in the after_request method)

Screenshots/GIFs

@Don-Leopardo Don-Leopardo requested a review from a team as a code owner April 27, 2024 02:37
@Don-Leopardo Don-Leopardo requested review from ankush and removed request for a team April 27, 2024 02:37
@github-actions github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Apr 27, 2024
@ankush ankush changed the title Fix: lstrip for query writes detection fix: lstrip for query writes detection Apr 30, 2024
@ankush ankush merged commit c0e7799 into frappe:develop Apr 30, 2024
23 of 24 checks passed
@ankush ankush added backport version-14-hotfix backport to version 14 backport version-15-hotfix Backport the PR to v15 and removed add-test-cases Add test case to validate fix or enhancement labels Apr 30, 2024
@ankush
Copy link
Member

ankush commented Apr 30, 2024

@Don-Leopardo this write count is unused since long time. Merging just for sanity.

#25322

mergify bot pushed a commit that referenced this pull request Apr 30, 2024
mergify bot pushed a commit that referenced this pull request Apr 30, 2024
ankush pushed a commit that referenced this pull request Apr 30, 2024
(cherry picked from commit c0e7799)

Co-authored-by: Nahuel Operto <46027152+Don-Leopardo@users.noreply.github.com>
ankush pushed a commit that referenced this pull request Apr 30, 2024
(cherry picked from commit c0e7799)

Co-authored-by: Nahuel Operto <46027152+Don-Leopardo@users.noreply.github.com>
frappe-pr-bot pushed a commit that referenced this pull request May 7, 2024
# [14.74.0](v14.73.0...v14.74.0) (2024-05-07)

### Bug Fixes

* Apply configured perms on address list ([#26334](#26334)) ([#26335](#26335)) ([4307ab4](4307ab4))
* args is a stringified JSON ([98ece0e](98ece0e))
* changes for scheduler reliability (backport [#26292](#26292)) ([#26293](#26293)) ([7691afe](7691afe))
* **Data Import:** don't rely on permission for Data Import Log (backport [#26228](#26228)) ([#26250](#26250)) ([fd0a844](fd0a844))
* **Data Import:** scheduler not needed in dev mode (backport [#24667](#24667)) ([#26264](#26264)) ([9712f14](9712f14))
* disabled user login from login via link feature ([#26134](#26134)) ([#26140](#26140)) ([96b7542](96b7542))
* don't add creation index if one exists ([#26295](#26295)) ([#26297](#26297)) ([c74dcbd](c74dcbd))
* **Geo:** change Canadian dates to ISO 8601 format ([351cd04](351cd04))
* init db conn for unbuffered cursor if not set ([#26220](#26220)) ([#26256](#26256)) ([04afefb](04afefb))
* lstrip for query writes detection ([#26180](#26180)) ([#26252](#26252)) ([6ebfe54](6ebfe54))
* multistep webform page navigation ([d5a25f2](d5a25f2))
* **Navbar Settings:** reload page after save ([#26274](#26274)) ([#26275](#26275)) ([73f265b](73f265b))
* **oauth2:** refresh token is optional ([#26266](#26266)) ([#26271](#26271)) ([d6603c6](d6603c6)), closes [/www.rfc-editor.org/rfc/rfc6749#section-5](https://github.com//www.rfc-editor.org/rfc/rfc6749/issues/section-5)
* only redirect to same domain (backport [#26304](#26304)) ([#26305](#26305)) ([c2f2d6c](c2f2d6c))
* perm query for dashboard (backport [#26239](#26239)) ([#26242](#26242)) ([4ab6a46](4ab6a46))
* reportview average of ints should be float (backport [#26284](#26284)) ([#26287](#26287)) ([c0f3912](c0f3912))
* Treeview DB lookup should perform the same preperation operations as method update_nsm in file nestedset.py ([#26199](#26199)) ([#26259](#26259)) ([01e08f8](01e08f8))

### Features

* `Desk User` role (backport [#22224](#22224)) ([#26237](#26237)) ([171e1d0](171e1d0))
* System Health Report (backport [#26046](#26046)) ([#26255](#26255)) ([f2d2d0c](f2d2d0c))

### Performance Improvements

* Reduce 1 redis call while dumping monitor logs (backport [#26337](#26337)) ([#26338](#26338)) ([75b2a86](75b2a86))
frappe-pr-bot pushed a commit that referenced this pull request May 7, 2024
# [15.26.0](v15.25.0...v15.26.0) (2024-05-07)

### Bug Fixes

* allow accessing reports without roles ([b2950d2](b2950d2))
* Apply configured perms on address list ([#26334](#26334)) ([#26336](#26336)) ([e2c8436](e2c8436))
* args is a stringified JSON ([5f25ae6](5f25ae6))
* automatic logo width ([#26273](#26273)) ([#26278](#26278)) ([f60be3c](f60be3c))
* changes for scheduler reliability (backport [#26292](#26292)) ([#26294](#26294)) ([d72c6ec](d72c6ec))
* **Data Import:** don't rely on permission for Data Import Log (backport [#26228](#26228)) ([#26251](#26251)) ([113de20](113de20))
* **Data Import:** scheduler not needed in dev mode (backport [#24667](#24667)) ([#26265](#26265)) ([9ce789e](9ce789e))
* disabled user login from login via link feature ([#26134](#26134)) ([#26141](#26141)) ([3ac1eda](3ac1eda))
* don't add creation index if one exists ([#26295](#26295)) ([f183a31](f183a31))
* **Geo:** change Canadian dates to ISO 8601 format ([9dec74b](9dec74b))
* init db conn for unbuffered cursor if not set ([#26220](#26220)) ([#26257](#26257)) ([1fac55d](1fac55d))
* like filter from meta filters ([#26311](#26311)) ([#26313](#26313)) ([13535a6](13535a6))
* lstrip for query writes detection ([#26180](#26180)) ([#26253](#26253)) ([97355f9](97355f9))
* misc sys health report fixes (backport [#26262](#26262)) ([#26290](#26290)) ([889eb9a](889eb9a))
* **Navbar Settings:** reload page after save ([#26274](#26274)) ([#26276](#26276)) ([20b3643](20b3643))
* **oauth2:** refresh token is optional ([#26266](#26266)) ([#26272](#26272)) ([e3f15d0](e3f15d0)), closes [/www.rfc-editor.org/rfc/rfc6749#section-5](https://github.com//www.rfc-editor.org/rfc/rfc6749/issues/section-5)
* only redirect to same domain ([#26304](#26304)) ([#26306](#26306)) ([475189c](475189c))
* QB name in error message ([#26345](#26345)) ([#26348](#26348)) ([6e8de81](6e8de81))
* reportview average of ints should be float ([#26284](#26284)) ([#26288](#26288)) ([11f041b](11f041b))
* Treeview DB lookup should perform the same preperation operations as method update_nsm in file nestedset.py ([#26199](#26199)) ([7d25aed](7d25aed))

### Features

* improved update notifications (backport [#26314](#26314)) ([#26320](#26320)) ([24be120](24be120))
* link field filter backport v15 ([#25966](#25966)) ([9a15191](9a15191)), closes [#22844](#22844)
* workspace extraction improvements ([#26169](#26169)) ([7d6a40a](7d6a40a))

### Performance Improvements

* Avoid caching `module_app` ([#26349](#26349)) ([#26351](#26351)) ([8961a96](8961a96))
* avoid two layer cache of timezone (backport [#26340](#26340)) ([#26342](#26342)) ([d802eb6](d802eb6))
* Reduce 1 redis call while dumping monitor logs ([#26337](#26337)) ([#26339](#26339)) ([32dbc5a](32dbc5a))
* Reuse cached user for `get_user_lang` ([#26350](#26350)) ([#26355](#26355)) ([e1048a8](e1048a8))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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

2 participants