Skip to content

chore(api): cache the setup status to cut down DB access#36966

Open
cqjjjzr wants to merge 5 commits into
langgenius:mainfrom
cqjjjzr:setup-cache
Open

chore(api): cache the setup status to cut down DB access#36966
cqjjjzr wants to merge 5 commits into
langgenius:mainfrom
cqjjjzr:setup-cache

Conversation

@cqjjjzr
Copy link
Copy Markdown
Contributor

@cqjjjzr cqjjjzr commented Jun 2, 2026

Important

  1. Make sure you have read our contribution guidelines
  2. Ensure there is an associated issue and you have been assigned to it
  3. Use the correct syntax to link this PR: Fixes #<issue number>.

Summary

Fix #36965 . Not using @cache due to reasons mentioned in the comment.s

Checklist

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran make lint && make type-check (backend) and cd web && pnpm exec vp staged (frontend) to appease the lint gods

@cqjjjzr cqjjjzr requested a review from a team June 2, 2026 17:39
@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jun 2, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-06-05 00:42:26.802000994 +0000
+++ /tmp/pyrefly_pr.txt	2026-06-05 00:42:15.570946821 +0000
@@ -2081,7 +2081,7 @@
 ERROR `SimpleNamespace` is not assignable to attribute `_current_tenant` with type `Tenant | None` [bad-assignment]
   --> tests/unit_tests/controllers/console/test_workspace_members.py:63:43
 ERROR `in` is not supported between `Literal['count']` and `None` [not-iterable]
-   --> tests/unit_tests/controllers/console/test_wraps.py:213:16
+   --> tests/unit_tests/controllers/console/test_wraps.py:220:16
 ERROR `SimpleNamespace` is not assignable to attribute `db` with type `SQLAlchemy` [bad-assignment]
   --> tests/unit_tests/controllers/files/test_image_preview.py:23:17
 ERROR `SimpleNamespace` is not assignable to attribute `request` with type `Request` [bad-assignment]

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

Pyrefly Type Coverage

Metric Base PR Delta
Type coverage 46.88% 46.89% +0.00%
Strict coverage 46.38% 46.38% +0.00%
Typed symbols 25,884 25,892 +8
Untyped symbols 29,644 29,650 +6
Modules 2819 2819 0

@autofix-ci autofix-ci Bot requested a review from crazywoola as a code owner June 2, 2026 17:43
@asukaminato0721
Copy link
Copy Markdown
Contributor

try use closure.

or class.

import functools

def once_true(func):
    has_success = False 
    
    @functools.wraps(func)
    def wrapper(*args, **kwargs):
        nonlocal has_success
        
        if has_success:
        
            return True
            
        result = func(*args, **kwargs)
        
        if result is True:
            has_success = True
            
        return result
        
    return wrapper

@asukaminato0721
Copy link
Copy Markdown
Contributor

@asukaminato0721 asukaminato0721 removed the request for review from a team June 3, 2026 05:37
@asukaminato0721 asukaminato0721 enabled auto-merge June 3, 2026 06:03
@cqjjjzr cqjjjzr requested a review from a team June 3, 2026 06:52
@asukaminato0721 asukaminato0721 removed the request for review from a team June 4, 2026 03:11
@asukaminato0721
Copy link
Copy Markdown
Contributor

@dosu how is this change

@asukaminato0721 asukaminato0721 self-assigned this Jun 4, 2026
@dosubot
Copy link
Copy Markdown

dosubot Bot commented Jun 4, 2026

This PR looks like a solid, well-thought-out optimization. Here's a quick review:

What it does well:

  1. Smart caching strategy — The once_true decorator only memoizes True results, which is exactly right for a one-way state transition (setup goes from incomplete → complete, never back). This avoids the footgun of @cache permanently memoizing an initial False before setup has run .

  2. Immediate cache warming — Calling mark_setup_completed() right after creating the DifySetup record avoids an unnecessary extra DB round-trip .

  3. Good test coverage — Tests verify both that True is cached (only 1 DB call for multiple checks) and that False is not cached .

  4. Real impact — Since setup_required decorates every protected console endpoint, this eliminates a SELECT on every single API request once the instance is set up.

Minor observations / suggestions:

  • The implementation already uses the closure-based approach that @asukaminato0721 suggested, with the addition of mark_success() / reset_success() helper methods attached to the wrapper — nice for testability and the explicit mark_setup_completed() path.
  • The cache is process-scoped (closure variable), so it naturally resets on server restart — no stale state concerns across deploys.
  • One edge case worth noting: if someone resets their Dify instance (purges DifySetup from the DB) without restarting the server, the cached True would persist. This is acknowledged in issue [Refactor/Chore] Cut down a DB access by caching setup status for setup_required #36965 and is an acceptable trade-off since that scenario is extremely rare and a restart would fix it.

Overall, this is a clean, minimal change that removes unnecessary DB load in the hot path. Looks good to merge. 👍

To reply, just mention @dosu.


Share context across your team and agents. Try Dosu.

Leave Feedback Learn about dify with Dosu

Comment thread api/controllers/console/wraps.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Refactor/Chore] Cut down a DB access by caching setup status for setup_required

2 participants