Skip to content

fix(site): Skip file-tree walk when refreshing only database usage #6765

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.

Sign up for GitHub

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

Open
balamurali27 wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

fix(site): Skip file-tree walk when refreshing only database usage #6765

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
834fe61
fix(site): Skip file-tree walk when refreshing only database usage
balamurali27 Jun 22, 2026
7f24318
fix(site): Type-hint database_only so whitelist coerces the string
balamurali27 Jun 22, 2026
da8b5b8
Merge branch 'develop' into fix/database-usage-skip-file-walk
balamurali27 Jun 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions press/agent.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1208,8 +1208,11 @@ def get_site_sid(self, site, user=None):
result = self.get(f"benches/{site.bench}/sites/{site.name}/sid")
return result and result.get("sid")

def get_site_info(self, site):
result = self.get(f"benches/{site.bench}/sites/{site.name}/info")
def get_site_info(self, site, database_only=False):
path = f"benches/{site.bench}/sites/{site.name}/info"
if database_only:
path += "?database_only=1"
result = self.get(path)
if result:
return result["data"]
return None
Expand Down
34 changes: 28 additions & 6 deletions press/press/doctype/site/site.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2065,9 +2065,9 @@ def get_login_sid(self, user: str = "Administrator"):
frappe.throw(f"Could not login as {user}", frappe.ValidationError) # nosemgrep
return sid

def fetch_info(self):
def fetch_info(self, database_only=False):
agent = Agent(self.server)
return agent.get_site_info(self)
return agent.get_site_info(self, database_only=database_only)

def fetch_analytics(self):
agent = Agent(self.server)
Expand Down Expand Up @@ -2107,6 +2107,24 @@ def _sync_config_info(self, fetched_config: dict) -> bool:
return True
return False

def _sync_database_usage(self, fetched_usage: dict):
"""Record a Site Usage row, refreshing only the database size.

Carries the last-known file sizes forward so a database-usage refresh
doesn't trigger the expensive file-tree walk in the agent's get_usage.
"""
last = self.get_disk_usages()
self._insert_site_usage(
{
"database": fetched_usage["database"],
Comment on lines +2117 to +2119

@greptile-apps greptile-apps Bot Jun 22, 2026

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 _sync_database_usage accesses fetched_usage["database"] with a hard key lookup. If the agent returns a response that passes the if not data guard but has a missing or renamed "database" key (e.g., during a partial rollout of the matching agent change), this raises an unhandled KeyError in the refresh_database_usage code path, where there is no suppress(Exception) wrapper — surfacing a 500 to the polling dashboard every 3 s.

Suggested change
self._insert_site_usage(
{
"database": fetched_usage["database"],
database_size = fetched_usage.get("database")
if database_size is None:
return
self._insert_site_usage(
{
"database": database_size,
Prompt To Fix With AI 
This is a comment left during a code review.
Path: press/press/doctype/site/site.py
Line: 2114-2116

Comment:
`_sync_database_usage` accesses `fetched_usage["database"]` with a hard key lookup. If the agent returns a response that passes the `if not data` guard but has a missing or renamed `"database"` key (e.g., during a partial rollout of the matching agent change), this raises an unhandled `KeyError` in the `refresh_database_usage` code path, where there is no `suppress(Exception)` wrapper — surfacing a 500 to the polling dashboard every 3 s.

```suggestion
		database_size = fetched_usage.get("database")
		if database_size is None:
			return
		self._insert_site_usage(
			{
				"database": database_size,
```

How can I resolve this? If you propose a fix, please make it concise.

"database_free": fetched_usage.get("database_free", 0),
"database_free_tables": fetched_usage.get("database_free_tables", []),
"public": last["public"] or 0,
"private": last["private"] or 0,
"backups": last["backups"] or 0,
}
)

def _sync_usage_info(self, fetched_usage: dict):
"""Generate a Site Usage doc for the site using the fetched_usage data.

Expand Down Expand Up @@ -2184,14 +2202,18 @@ def _sync_database_name(self, config):
return False

@frappe.whitelist()
def sync_info(self, data=None):
def sync_info(self, data=None, database_only: bool = False):
"""Updates Site Usage, site.config and timezone details for site."""
if not data:
data = self.fetch_info()
data = self.fetch_info(database_only=database_only)

if not data:
return

if database_only:
self._sync_database_usage(data["usage"])
return

fetched_usage = data["usage"]
fetched_config = data["config"]
fetched_timezone = data["timezone"]
Expand Down Expand Up @@ -3942,7 +3964,7 @@ def add_database_index(self, table, column):
def refresh_database_usage(self):
# Check if schema parser enabled on db server
if not frappe.db.get_value("Database Server", self.database_server_name, "enable_schema_size_parser"):
self.sync_info()
self.sync_info(database_only=True)
return {
"synced": True,
}
Expand Down Expand Up @@ -5498,7 +5520,7 @@ def process_refresh_database_usage_job_update(job: AgentJob):
site: Site = frappe.get_doc("Site", job.site)
with suppress(Exception):
# Don't throw error on failure of syncing also
site.sync_info()
site.sync_info(database_only=True)


def on_doctype_update():
Expand Down
25 changes: 25 additions & 0 deletions press/press/doctype/site/test_site.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,31 @@ def test_site_usage_exceed_tracking(self):
self.assertIsNotNone(site.site_usage_exceeded_on)
self.assertEqual(site.status, "Active")

def test_sync_info_database_only_refreshes_db_size_and_carries_files_forward(self):
site = create_test_site()
frappe.get_doc(
{
"doctype": "Site Usage",
"site": site.name,
"database": 100,
"public": 200,
"private": 300,
"backups": 400,
}
).insert()

# In database_only mode the agent returns only the database size; the
# file totals must be carried forward, not recomputed (no file walk).
with patch.object(Site, "fetch_info", return_value={"usage": {"database": 150}}) as fetch_info:
site.sync_info(database_only=True)

fetch_info.assert_called_once_with(database_only=True)
latest = frappe.get_last_doc("Site Usage", {"site": site.name})
self.assertEqual(latest.database, 150)
self.assertEqual(latest.public, 200)
self.assertEqual(latest.private, 300)
self.assertEqual(latest.backups, 400)

def test_free_sites_ignore_usage_exceed_tracking(self):
team = create_test_team(free_account=False)
plan_10 = create_test_plan("Site", price_usd=10.0, price_inr=750.0, plan_name="USD 10")
Expand Down
Loading