-
Notifications
You must be signed in to change notification settings - Fork 373
feat: background job system (tracked async jobs + jobs UI) #1435
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.
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
leonardmq
wants to merge
33
commits into
main
Choose a base branch
from
leonard/kil-686-feat-background-job-system
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+6,520
−21
Open
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
8290cfe
feat: background job system
leonardmq 90b200d
harness for testing eval running through job
leonardmq aed9af9
chore: build annotations
leonardmq 509e160
feat: support waiting for job
leonardmq 18d5988
feat: job widget + dialog
leonardmq 45c17ec
refactor: link in sidebar rail
leonardmq fc5f6f6
refactor: job entry in sidebar
leonardmq 34f10db
fix: guard task.uncancel() for Python 3.10 in job registry tests
leonardmq 180224c
Merge branch 'main' of github.com:Kiln-AI/Kiln into leonard/kil-686-f…
leonardmq 589d8dd
fix: address PR review on background job system
leonardmq 1b19939
feat: typed per-worker progress payload
leonardmq a14d73f
chore: fmt
leonardmq ecc3e58
Merge pull request #1463 from Kiln-AI/leonard/kil-686-jobs-typed-payl…
leonardmq b2ccf5f
Merge branch 'main' of github.com:Kiln-AI/Kiln into leonard/kil-686-f…
leonardmq 8b9199f
feat: rename sidebar "In progress" to "Jobs" with active-state styling
leonardmq ad9c241
fix: harden job reconcile + offload eval compute_state IO
leonardmq 824edec
fix: keep jobs SSE stream alive past keepalive + close it on shutdown
leonardmq 619a915
refactor: ui redesign for jobs
leonardmq a440f19
fix: drop unsatisfiable networkidle wait in docs-library e2e test
leonardmq 406ce32
chore: delete spec project
leonardmq 3a33b65
Merge branch 'main' of github.com:Kiln-AI/Kiln into leonard/kil-686-f…
leonardmq 7a63c89
Merge branch 'main' of github.com:Kiln-AI/Kiln into leonard/kil-686-f…
leonardmq 922eff4
refactor: use intro component for job table
leonardmq 47b24c4
refactor: three dots dropdown instead of buttons
leonardmq aa9de8b
refactor: one string subtitle in modal
leonardmq 424e4fb
refactor: move jobs entry to below App Update Available in sidebar
leonardmq b2040c3
fix: maybe fix wrapping badge
leonardmq b254de3
Merge branch 'main' of github.com:Kiln-AI/Kiln into leonard/kil-686-f…
leonardmq 97cf700
refactor: tighten up jobs empty-state copy
leonardmq d978693
feat: gate Jobs sidebar behind PUBLIC_ENABLE_JOBS flag
leonardmq 53d75bd
chore: remove Run eval example job from jobs system
leonardmq 887bcc0
test: update jobs_table tests for 3-dot dropdown actions
leonardmq 749d951
Merge branch 'main' of github.com:Kiln-AI/Kiln into leonard/kil-686-f…
leonardmq File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from pathlib import Path | ||
|
|
||
| from kiln_ai.utils.git_sync_protocols import SaveContext | ||
|
|
||
| from app.desktop.git_sync.config import get_git_sync_config, project_path_from_id | ||
| from app.desktop.git_sync.git_sync_manager import GitSyncManager | ||
| from app.desktop.git_sync.registry import GitSyncRegistry | ||
|
|
||
|
|
||
| def get_manager_for_project(project_id: str) -> GitSyncManager | None: | ||
| """Resolve a project_id to its GitSyncManager when auto-sync is active. | ||
|
|
||
| Request-free mirror of GitSyncMiddleware._get_manager_for_request (minus the | ||
| URL parsing). Returns None for every "not active" branch: the project has no | ||
| path, no git-sync config, sync_mode is not "auto", or no clone_path is set. | ||
|
|
||
| Config is keyed by project_path; the manager is keyed by clone_path. The | ||
| manager is always obtained via GitSyncRegistry.get_or_create so the single | ||
| per-clone-path manager (and its executor + non-reentrant write lock) is | ||
| shared with the HTTP path. | ||
| """ | ||
| project_path = project_path_from_id(project_id) | ||
| if project_path is None: | ||
| return None | ||
|
|
||
| config = get_git_sync_config(project_path) | ||
| if config is None: | ||
| return None | ||
|
|
||
| if config["sync_mode"] != "auto": | ||
| return None | ||
|
|
||
| clone_path = config.get("clone_path") | ||
| if clone_path is None: | ||
| return None | ||
|
|
||
| return GitSyncRegistry.get_or_create( | ||
| repo_path=Path(clone_path), | ||
| remote_name=config["remote_name"], | ||
| pat_token=config.get("pat_token"), | ||
| oauth_token=config.get("oauth_token"), | ||
| auth_mode=config["auth_mode"], | ||
| ) | ||
|
|
||
|
|
||
| def save_context_for_project(project_id: str, context: str) -> SaveContext | None: | ||
| """Return a SaveContext wrapping writes in manager.atomic_write(context=...), | ||
| or None when git sync is not active for this project. | ||
|
|
||
| Mirrors build_save_context(request) for callers that have only a project_id | ||
| (e.g. background job workers). Runners coalesce None to a no-op context. | ||
| """ | ||
| manager = get_manager_for_project(project_id) | ||
| if manager is None: | ||
| return None | ||
|
|
||
| bg_sync = GitSyncRegistry.get_background_sync(manager.repo_path) | ||
| if bg_sync is not None: | ||
| bg_sync.notify_request() | ||
|
|
||
| def factory(): | ||
| return manager.atomic_write(context=context) | ||
|
|
||
| return factory | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,219 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from contextlib import ExitStack, asynccontextmanager | ||
| from pathlib import Path | ||
| from unittest.mock import MagicMock, patch | ||
|
|
||
| import pytest | ||
|
|
||
| from app.desktop.git_sync.config import GitSyncProjectConfig | ||
| from app.desktop.git_sync.save_context import ( | ||
| get_manager_for_project, | ||
| save_context_for_project, | ||
| ) | ||
|
|
||
| PROJECT_ID = "project_abc" | ||
| PROJECT_PATH = "/tmp/test/project.kiln" | ||
| CLONE_PATH = "/tmp/test/clone" | ||
|
|
||
|
|
||
| def _auto_config(clone_path: str | None = CLONE_PATH) -> GitSyncProjectConfig: | ||
| return GitSyncProjectConfig( | ||
| sync_mode="auto", | ||
| auth_mode="system_keys", | ||
| remote_name="origin", | ||
| branch="main", | ||
| clone_path=clone_path, | ||
| git_url=None, | ||
| pat_token=None, | ||
| oauth_token=None, | ||
| ) | ||
|
|
||
|
|
||
| def _manual_config() -> GitSyncProjectConfig: | ||
| return GitSyncProjectConfig( | ||
| sync_mode="manual", | ||
| auth_mode="system_keys", | ||
| remote_name="origin", | ||
| branch="main", | ||
| clone_path=CLONE_PATH, | ||
| git_url=None, | ||
| pat_token=None, | ||
| oauth_token=None, | ||
| ) | ||
|
|
||
|
|
||
| class _FakeManager: | ||
| """Minimal AtomicWriteCapable stand-in that records atomic_write calls.""" | ||
|
|
||
| def __init__(self, repo_path: Path = Path(CLONE_PATH)): | ||
| self.repo_path = repo_path | ||
| self.calls: list[str] = [] | ||
| self.entered = False | ||
|
|
||
| @asynccontextmanager | ||
| async def atomic_write(self, context: str): | ||
| self.calls.append(context) | ||
| self.entered = True | ||
| yield | ||
|
|
||
|
|
||
| def _patch_resolution(project_path, config, manager=None, bg_sync=None): | ||
| """Patch the config + registry calls used by the helper. | ||
|
|
||
| project_path_from_id and get_git_sync_config are looked up in the | ||
| save_context module namespace, so patch them there. | ||
| """ | ||
| stack = ExitStack() | ||
| stack.enter_context( | ||
| patch( | ||
| "app.desktop.git_sync.save_context.project_path_from_id", | ||
| return_value=project_path, | ||
| ) | ||
| ) | ||
| stack.enter_context( | ||
| patch( | ||
| "app.desktop.git_sync.save_context.get_git_sync_config", | ||
| return_value=config, | ||
| ) | ||
| ) | ||
| stack.enter_context( | ||
| patch( | ||
| "app.desktop.git_sync.save_context.GitSyncRegistry.get_or_create", | ||
| return_value=manager, | ||
| ) | ||
| ) | ||
| stack.enter_context( | ||
| patch( | ||
| "app.desktop.git_sync.save_context.GitSyncRegistry.get_background_sync", | ||
| return_value=bg_sync, | ||
| ) | ||
| ) | ||
| return stack | ||
|
|
||
|
|
||
| # -- None branches ----------------------------------------------------------- | ||
|
|
||
|
|
||
| def test_returns_none_when_no_project_path(): | ||
| with _patch_resolution(project_path=None, config=None): | ||
| assert save_context_for_project(PROJECT_ID, context="ctx") is None | ||
| assert get_manager_for_project(PROJECT_ID) is None | ||
|
|
||
|
|
||
| def test_returns_none_when_no_git_sync_config(): | ||
| with _patch_resolution(project_path=PROJECT_PATH, config=None): | ||
| assert save_context_for_project(PROJECT_ID, context="ctx") is None | ||
| assert get_manager_for_project(PROJECT_ID) is None | ||
|
|
||
|
|
||
| def test_returns_none_when_sync_mode_not_auto(): | ||
| with _patch_resolution(project_path=PROJECT_PATH, config=_manual_config()): | ||
| assert save_context_for_project(PROJECT_ID, context="ctx") is None | ||
| assert get_manager_for_project(PROJECT_ID) is None | ||
|
|
||
|
|
||
| def test_returns_none_when_clone_path_missing(): | ||
| with _patch_resolution( | ||
| project_path=PROJECT_PATH, config=_auto_config(clone_path=None) | ||
| ): | ||
| assert save_context_for_project(PROJECT_ID, context="ctx") is None | ||
| assert get_manager_for_project(PROJECT_ID) is None | ||
|
|
||
|
|
||
| # -- active branches --------------------------------------------------------- | ||
|
|
||
|
|
||
| def test_get_manager_uses_registry_with_config_values(): | ||
| manager = _FakeManager() | ||
| with ( | ||
| patch( | ||
| "app.desktop.git_sync.save_context.project_path_from_id", | ||
| return_value=PROJECT_PATH, | ||
| ), | ||
| patch( | ||
| "app.desktop.git_sync.save_context.get_git_sync_config", | ||
| return_value=_auto_config(), | ||
| ), | ||
| patch( | ||
| "app.desktop.git_sync.save_context.GitSyncRegistry.get_or_create", | ||
| return_value=manager, | ||
| ) as mock_get_or_create, | ||
| ): | ||
| result = get_manager_for_project(PROJECT_ID) | ||
|
|
||
| assert result is manager | ||
| mock_get_or_create.assert_called_once_with( | ||
| repo_path=Path(CLONE_PATH), | ||
| remote_name="origin", | ||
| pat_token=None, | ||
| oauth_token=None, | ||
| auth_mode="system_keys", | ||
| ) | ||
|
|
||
|
|
||
| async def test_save_context_enters_atomic_write_with_label(): | ||
| manager = _FakeManager() | ||
| with _patch_resolution( | ||
| project_path=PROJECT_PATH, config=_auto_config(), manager=manager | ||
| ): | ||
| save_context = save_context_for_project(PROJECT_ID, context="eval job e1/r1") | ||
|
|
||
| assert save_context is not None | ||
| assert manager.entered is False # built lazily, not yet entered | ||
|
|
||
| async with save_context(): | ||
| pass | ||
|
|
||
| assert manager.calls == ["eval job e1/r1"] | ||
|
|
||
|
|
||
| def test_save_context_notifies_background_sync(): | ||
| manager = _FakeManager() | ||
| bg_sync = MagicMock() | ||
| with _patch_resolution( | ||
| project_path=PROJECT_PATH, | ||
| config=_auto_config(), | ||
| manager=manager, | ||
| bg_sync=bg_sync, | ||
| ): | ||
| save_context = save_context_for_project(PROJECT_ID, context="ctx") | ||
|
|
||
| assert save_context is not None | ||
| bg_sync.notify_request.assert_called_once() | ||
|
|
||
|
|
||
| def test_save_context_no_background_sync_is_fine(): | ||
| manager = _FakeManager() | ||
| with _patch_resolution( | ||
| project_path=PROJECT_PATH, | ||
| config=_auto_config(), | ||
| manager=manager, | ||
| bg_sync=None, | ||
| ): | ||
| save_context = save_context_for_project(PROJECT_ID, context="ctx") | ||
|
|
||
| assert save_context is not None | ||
|
|
||
|
|
||
| # -- error propagation ------------------------------------------------------- | ||
|
|
||
|
|
||
| def test_propagates_when_config_lookup_raises(): | ||
| # A corrupt/raising config lookup must surface (failing the job) rather than | ||
| # be swallowed to None, which would silently skip commits for an auto-sync | ||
| # project — the very bug this resolver exists to prevent. | ||
| with ( | ||
| patch( | ||
| "app.desktop.git_sync.save_context.project_path_from_id", | ||
| return_value=PROJECT_PATH, | ||
| ), | ||
| patch( | ||
| "app.desktop.git_sync.save_context.get_git_sync_config", | ||
| side_effect=RuntimeError("corrupt config"), | ||
| ), | ||
| ): | ||
| with pytest.raises(RuntimeError, match="corrupt config"): | ||
| get_manager_for_project(PROJECT_ID) | ||
| with pytest.raises(RuntimeError, match="corrupt config"): | ||
| save_context_for_project(PROJECT_ID, context="ctx") |
Empty file.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.