Skip to content

refactor: Route all GET endpoints to read replica via read_only - #3716

Open
niraj2477 wants to merge 1 commit into
frappe:developfrom
niraj2477:feat/route-to-read-replica
Open

refactor: Route all GET endpoints to read replica via read_only#3716
niraj2477 wants to merge 1 commit into
frappe:developfrom
niraj2477:feat/route-to-read-replica

Conversation

@niraj2477

Copy link
Copy Markdown
Contributor

This pr routes all the GET endpoint via read_only wrapper so that the read_replica can be used for query.

@greptile-apps

greptile-apps Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 4/5

This should be fixed before merging because a newly created ticket can immediately appear missing during replication lag.

Ticket creation redirects directly to a detail endpoint that now reads from the replica and explicitly fails when the replicated row is not yet visible.

Files Needing Attention: helpdesk/helpdesk/doctype/hd_ticket/api.py

Prompt To Fix All With AI
### Issue 1
helpdesk/helpdesk/doctype/hd_ticket/api.py:39
**Replica hides new tickets**

If replication is delayed, the post-creation redirect immediately calls `get_one` against the replica before the new ticket is visible, causing the customer’s successfully created ticket page to fail with “Ticket not found.”

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "refactor: Route all GET endpoints to rea..." | Re-trigger Greptile



@frappe.whitelist()
@frappe.read_only()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Replica hides new tickets

If replication is delayed, the post-creation redirect immediately calls get_one against the replica before the new ticket is visible, causing the customer’s successfully created ticket page to fail with “Ticket not found.”

Knowledge Base Used: Ticket management

Prompt To Fix With AI
This is a comment left during a code review.
Path: helpdesk/helpdesk/doctype/hd_ticket/api.py
Line: 39

Comment:
**Replica hides new tickets**

If replication is delayed, the post-creation redirect immediately calls `get_one` against the replica before the new ticket is visible, causing the customer’s successfully created ticket page to fail with “Ticket not found.”

**Knowledge Base Used:** [Ticket management](https://app.greptile.com/frappe/-/custom-context/knowledge-base/frappe/helpdesk/-/docs/ticket-management.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Replication is handled by DB, and did not came across such case in high traffic system, If still required this can be updated atleast for this.

@RitvikSardana

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That’s fair—this may be rare in practice, but asynchronous DB replication still allows a newly inserted ticket to be temporarily absent from the replica. Since get_one is commonly called immediately after new, please leave get_one off @frappe.read_only() (or explicitly route that post-create fetch to the primary). The other read endpoints can remain covered by this change.

Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.

@mergify

mergify Bot commented Aug 26, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.44068% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.87%. Comparing base (c3ae467) to head (a5ee0e7).
⚠️ Report is 5 commits behind head on develop.

Files with missing lines Patch % Lines
helpdesk/api/article.py 0.00% 2 Missing ⚠️
helpdesk/api/search.py 0.00% 2 Missing ⚠️
helpdesk/api/assignment_rule.py 0.00% 1 Missing ⚠️
helpdesk/api/config.py 0.00% 1 Missing ⚠️
helpdesk/api/general.py 0.00% 1 Missing ⚠️
helpdesk/api/session.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3716      +/-   ##
===========================================
+ Coverage    73.76%   73.87%   +0.10%     
===========================================
  Files          148      148              
  Lines        10841    10904      +63     
===========================================
+ Hits          7997     8055      +58     
- Misses        2844     2849       +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@niraj2477

Copy link
Copy Markdown
Contributor Author

@RitvikSardana Can you check if this can be merged?

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