fix(dashboard): async git status enrichment — await get_repo()
_get_git_status_for_dashboard was sync but called async git_service.get_repo() without await. Coroutine was always truthy, so active_branch access failed silently and returned None. Made function async, added await, updated tests.
This commit is contained in:
@@ -67,7 +67,7 @@ class ResourceService:
|
||||
|
||||
# Git status can be skipped for list endpoints and loaded lazily on UI side.
|
||||
if include_git_status:
|
||||
git_status = self._get_git_status_for_dashboard(dashboard_id)
|
||||
git_status = await self._get_git_status_for_dashboard(dashboard_id)
|
||||
dashboard_dict['git_status'] = git_status
|
||||
else:
|
||||
dashboard_dict['git_status'] = None
|
||||
@@ -82,7 +82,7 @@ class ResourceService:
|
||||
|
||||
result.append(dashboard_dict)
|
||||
|
||||
logger.info(f"[ResourceService][Coherence:OK] Fetched {len(result)} dashboards with status")
|
||||
logger.reflect("Fetched dashboards with status", payload={"count": len(result)})
|
||||
return result
|
||||
# endregion get_dashboards_with_status
|
||||
|
||||
@@ -126,7 +126,7 @@ class ResourceService:
|
||||
dashboard_id = dashboard_dict.get("id")
|
||||
|
||||
if include_git_status:
|
||||
dashboard_dict["git_status"] = self._get_git_status_for_dashboard(dashboard_id)
|
||||
dashboard_dict["git_status"] = await self._get_git_status_for_dashboard(dashboard_id)
|
||||
else:
|
||||
dashboard_dict["git_status"] = None
|
||||
|
||||
@@ -138,12 +138,9 @@ class ResourceService:
|
||||
result.append(dashboard_dict)
|
||||
|
||||
total_pages = (total + page_size - 1) // page_size if total > 0 else 1
|
||||
logger.info(
|
||||
"[ResourceService][Coherence:OK] Fetched dashboards page %s/%s (%s items, total=%s)",
|
||||
page,
|
||||
total_pages,
|
||||
len(result),
|
||||
total,
|
||||
logger.reflect(
|
||||
"Fetched dashboards page",
|
||||
payload={"page": page, "total_pages": total_pages, "count": len(result), "total": total},
|
||||
)
|
||||
return {
|
||||
"dashboards": result,
|
||||
@@ -318,9 +315,10 @@ class ResourceService:
|
||||
timeout=10.0,
|
||||
)
|
||||
except (TimeoutError, Exception):
|
||||
logger.warning(
|
||||
f"[EXT:method:get_datasets_with_status][Warning] "
|
||||
f"Failed to fetch linked dashboard count for dataset {ds_id}"
|
||||
logger.explore(
|
||||
"Failed to fetch linked dashboard count for dataset",
|
||||
payload={"dataset_id": ds_id},
|
||||
error="Timeout or API error",
|
||||
)
|
||||
return 0
|
||||
|
||||
@@ -346,9 +344,9 @@ class ResourceService:
|
||||
for ds, count in zip(result, linked_counts):
|
||||
ds['linked_dashboard_count'] = count
|
||||
|
||||
logger.info(
|
||||
f"[ResourceService][Coherence:OK] "
|
||||
f"Fetched {len(result)} datasets with status and linked counts"
|
||||
logger.reflect(
|
||||
"Fetched datasets with status and linked counts",
|
||||
payload={"count": len(result)},
|
||||
)
|
||||
return result
|
||||
# endregion get_datasets_with_status
|
||||
@@ -397,31 +395,19 @@ class ResourceService:
|
||||
# region _get_git_status_for_dashboard [TYPE Function]
|
||||
# @PURPOSE: Get Git sync status for a dashboard
|
||||
# @PRE dashboard_id is a valid integer
|
||||
# @POST Returns git status or None if no repo exists
|
||||
# @POST Returns git status dict (never None)
|
||||
# @PARAM dashboard_id (int) - The dashboard ID
|
||||
# @RETURN Optional[Dict] - Git status with branch and sync_status
|
||||
# @RETURN Dict - Git status with branch and sync_status
|
||||
# @RELATION CALLS ->[EXT:method:get_repo]
|
||||
def _get_git_status_for_dashboard(self, dashboard_id: int) -> dict[str, Any] | None:
|
||||
async def _get_git_status_for_dashboard(self, dashboard_id: int) -> dict[str, Any]:
|
||||
try:
|
||||
repo = self.git_service.get_repo(dashboard_id)
|
||||
if not repo:
|
||||
return {
|
||||
'branch': None,
|
||||
'sync_status': 'NO_REPO',
|
||||
'has_repo': False,
|
||||
'has_changes_for_commit': False
|
||||
}
|
||||
repo = await self.git_service.get_repo(dashboard_id)
|
||||
|
||||
# Check if there are uncommitted changes
|
||||
try:
|
||||
# Get current branch
|
||||
branch = repo.active_branch.name
|
||||
|
||||
# Check for uncommitted changes
|
||||
is_dirty = repo.is_dirty()
|
||||
has_changes_for_commit = repo.is_dirty(untracked_files=True)
|
||||
|
||||
# Check for unpushed commits
|
||||
unpushed = len(list(repo.iter_commits(f'{branch}@{{u}}..{branch}'))) if '@{u}' in str(repo.refs) else 0
|
||||
|
||||
if is_dirty or unpushed > 0:
|
||||
@@ -436,7 +422,7 @@ class ResourceService:
|
||||
'has_changes_for_commit': has_changes_for_commit
|
||||
}
|
||||
except Exception:
|
||||
logger.warning(f"[ResourceService][Warning] Failed to get git status for dashboard {dashboard_id}")
|
||||
logger.explore("Failed to get git status for dashboard", payload={"dashboard_id": dashboard_id}, error="Git operation failed")
|
||||
return {
|
||||
'branch': None,
|
||||
'sync_status': 'ERROR',
|
||||
@@ -444,7 +430,7 @@ class ResourceService:
|
||||
'has_changes_for_commit': False
|
||||
}
|
||||
except Exception:
|
||||
# No repo exists for this dashboard
|
||||
# No repo exists for this dashboard (HTTPException 404 from get_repo)
|
||||
return {
|
||||
'branch': None,
|
||||
'sync_status': 'NO_REPO',
|
||||
|
||||
@@ -362,62 +362,67 @@ class TestGetLastLlmTaskValidationStatusFallback:
|
||||
class TestGetGitStatusForDashboard:
|
||||
"""_get_git_status_for_dashboard — git repo status enrichment."""
|
||||
|
||||
def test_no_repo(self):
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_repo(self):
|
||||
from src.services.resource_service import ResourceService
|
||||
from fastapi import HTTPException
|
||||
svc = ResourceService()
|
||||
svc.git_service = MagicMock()
|
||||
svc.git_service.get_repo.return_value = None
|
||||
result = svc._get_git_status_for_dashboard(42)
|
||||
svc.git_service.get_repo = AsyncMock(side_effect=HTTPException(status_code=404))
|
||||
result = await svc._get_git_status_for_dashboard(42)
|
||||
assert result["sync_status"] == "NO_REPO"
|
||||
assert result["has_repo"] is False
|
||||
|
||||
def test_repo_error(self):
|
||||
@pytest.mark.asyncio
|
||||
async def test_repo_error(self):
|
||||
from src.services.resource_service import ResourceService
|
||||
svc = ResourceService()
|
||||
svc.git_service = MagicMock()
|
||||
svc.git_service.get_repo.side_effect = Exception("Git error")
|
||||
result = svc._get_git_status_for_dashboard(42)
|
||||
svc.git_service.get_repo = AsyncMock(side_effect=Exception("Git error"))
|
||||
result = await svc._get_git_status_for_dashboard(42)
|
||||
assert result["sync_status"] == "NO_REPO"
|
||||
|
||||
def test_git_operations_error(self):
|
||||
@pytest.mark.asyncio
|
||||
async def test_git_operations_error(self):
|
||||
from src.services.resource_service import ResourceService
|
||||
svc = ResourceService()
|
||||
repo = MagicMock()
|
||||
repo.active_branch.name = "main"
|
||||
repo.is_dirty.side_effect = Exception("Dirty check failed")
|
||||
svc.git_service = MagicMock()
|
||||
svc.git_service.get_repo.return_value = repo
|
||||
result = svc._get_git_status_for_dashboard(42)
|
||||
svc.git_service.get_repo = AsyncMock(return_value=repo)
|
||||
result = await svc._get_git_status_for_dashboard(42)
|
||||
assert result["sync_status"] == "ERROR"
|
||||
assert result["has_repo"] is True
|
||||
|
||||
def test_clean_repo(self):
|
||||
@pytest.mark.asyncio
|
||||
async def test_clean_repo(self):
|
||||
from src.services.resource_service import ResourceService
|
||||
svc = ResourceService()
|
||||
repo = MagicMock()
|
||||
repo.active_branch.name = "main"
|
||||
repo.is_dirty.return_value = False
|
||||
svc.git_service = MagicMock()
|
||||
svc.git_service.get_repo.return_value = repo
|
||||
# Mock gitpython's iter_commits functionality
|
||||
svc.git_service.get_repo = AsyncMock(return_value=repo)
|
||||
repo.iter_commits.return_value = []
|
||||
|
||||
result = svc._get_git_status_for_dashboard(42)
|
||||
result = await svc._get_git_status_for_dashboard(42)
|
||||
assert result["sync_status"] == "OK"
|
||||
assert result["has_repo"] is True
|
||||
assert result["branch"] == "main"
|
||||
|
||||
def test_dirty_repo(self):
|
||||
@pytest.mark.asyncio
|
||||
async def test_dirty_repo(self):
|
||||
from src.services.resource_service import ResourceService
|
||||
svc = ResourceService()
|
||||
repo = MagicMock()
|
||||
repo.active_branch.name = "main"
|
||||
repo.is_dirty.return_value = True
|
||||
svc.git_service = MagicMock()
|
||||
svc.git_service.get_repo.return_value = repo
|
||||
svc.git_service.get_repo = AsyncMock(return_value=repo)
|
||||
repo.iter_commits.return_value = []
|
||||
|
||||
result = svc._get_git_status_for_dashboard(42)
|
||||
result = await svc._get_git_status_for_dashboard(42)
|
||||
assert result["sync_status"] == "DIFF"
|
||||
assert result["has_changes_for_commit"] is True
|
||||
|
||||
@@ -430,7 +435,7 @@ class TestGetDashboardsWithStatus:
|
||||
def service(self):
|
||||
from src.services.resource_service import ResourceService
|
||||
svc = ResourceService()
|
||||
svc._get_git_status_for_dashboard = MagicMock(return_value={"sync_status": "OK"})
|
||||
svc._get_git_status_for_dashboard = AsyncMock(return_value={"sync_status": "OK"})
|
||||
svc._get_last_llm_task_for_dashboard = MagicMock(return_value={"status": "PASS"})
|
||||
return svc
|
||||
|
||||
@@ -470,7 +475,7 @@ class TestGetDashboardsPageWithStatus:
|
||||
def service(self):
|
||||
from src.services.resource_service import ResourceService
|
||||
svc = ResourceService()
|
||||
svc._get_git_status_for_dashboard = MagicMock(return_value={"sync_status": "OK"})
|
||||
svc._get_git_status_for_dashboard = AsyncMock(return_value={"sync_status": "OK"})
|
||||
svc._get_last_llm_task_for_dashboard = MagicMock(return_value={"status": "PASS"})
|
||||
return svc
|
||||
|
||||
|
||||
Reference in New Issue
Block a user