Skip to content

fix(observability): store usage audit sqlite under system data#2149

Merged
zhoujh01 merged 1 commit into
mainfrom
fix/usage-audit-system-path
May 20, 2026
Merged

fix(observability): store usage audit sqlite under system data#2149
zhoujh01 merged 1 commit into
mainfrom
fix/usage-audit-system-path

Conversation

@qin-ctx
Copy link
Copy Markdown
Collaborator

@qin-ctx qin-ctx commented May 20, 2026

Description

Move the default Usage/Audit SQLite database from the workspace root into the workspace _system area so runtime data follows the same internal-storage layout as QueueFS.

Related Issue

N/A

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • Updated the default Usage/Audit SQLite path to <workspace>/_system/usage_audit/usage_audit.sqlite3.
  • Kept explicit server.observability.usage_audit.sqlite_path behavior unchanged.
  • Updated Usage/Audit README default-path documentation.

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Ran:

.venv/bin/python -m py_compile openviking/observability/usage_audit/runtime.py

Note: local pre-commit hook could not run because it is installed against system Python without the pre_commit module.

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

N/A

Additional Notes

This change does not add legacy path migration because Usage/Audit has not been formally released yet.

@github-actions
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🏅 Score: 85
🧪 No relevant tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Missing Directory Creation

The new default SQLite path includes nested directories (_system/usage_audit/), but the code does not ensure these directories exist before opening the database. This will cause a failure when the SQLite store attempts to create the database file if the parent directories are missing.

_DEFAULT_SQLITE_RELATIVE_PATH = Path("_system") / "usage_audit" / "usage_audit.sqlite3"


@dataclass(slots=True)
class UsageAuditRuntime:
    """Container for active Usage/Audit runtime objects."""

    store: UsageAuditStore
    worker: UsageAuditWorker
    api_service: UsageAuditQueryService
    shutdown_flush_timeout_seconds: float


def _resolve_sqlite_path(config: ServerConfig) -> Path:
    configured = config.observability.usage_audit.sqlite_path
    if configured:
        return Path(configured).expanduser().resolve()
    try:
        ov_config = get_openviking_config()
        workspace = Path(ov_config.storage.workspace).expanduser().resolve()
    except Exception:  # noqa: BLE001
        workspace = DEFAULT_CONFIG_DIR
    return workspace / _DEFAULT_SQLITE_RELATIVE_PATH

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Create parent directories for SQLite path

Ensure parent directories for the SQLite path exist, both for the configured and
default cases, to avoid FileNotFoundError when opening the database. Use
mkdir(parents=True, exist_ok=True) on the path's parent.

openviking/observability/usage_audit/runtime.py [40-49]

 def _resolve_sqlite_path(config: ServerConfig) -> Path:
     configured = config.observability.usage_audit.sqlite_path
     if configured:
-        return Path(configured).expanduser().resolve()
-    try:
-        ov_config = get_openviking_config()
-        workspace = Path(ov_config.storage.workspace).expanduser().resolve()
-    except Exception:  # noqa: BLE001
-        workspace = DEFAULT_CONFIG_DIR
-    return workspace / _DEFAULT_SQLITE_RELATIVE_PATH
+        path = Path(configured).expanduser().resolve()
+    else:
+        try:
+            ov_config = get_openviking_config()
+            workspace = Path(ov_config.storage.workspace).expanduser().resolve()
+        except Exception:  # noqa: BLE001
+            workspace = DEFAULT_CONFIG_DIR
+        path = workspace / _DEFAULT_SQLITE_RELATIVE_PATH
+    path.parent.mkdir(parents=True, exist_ok=True)
+    return path
Suggestion importance[1-10]: 7

__

Why: The suggestion addresses a possible FileNotFoundError by ensuring parent directories exist for the SQLite path, which is particularly relevant with the new deeper default path. The improved code correctly refactors the function to create the parent directory for both configured and default cases.

Medium

@zhoujh01 zhoujh01 merged commit 1e484c9 into main May 20, 2026
5 checks passed
@zhoujh01 zhoujh01 deleted the fix/usage-audit-system-path branch May 20, 2026 13:20
@github-project-automation github-project-automation Bot moved this from Backlog to Done in OpenViking project May 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants