From fe75d4e980b107f783ae61f5d14b223702d02278 Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Mon, 24 Aug 2026 03:37:01 -0700 Subject: [PATCH 1/3] Read a namespace's default branch off the row that carries it Branch namespaces carry their own `github_repo_path`, so resolution stopped at the branch row and read `default_branch` from a row that never has one. `is_default_branch` came back false for the very namespace that tracks the repo's default branch, and the delete guard in `api/namespaces.py` let a git-backed default-branch namespace be hard-deleted. `default_branch` now comes from the first ancestor that has one, in the same most-specific-first order the repo path and the git branch already use, and that order includes the FK parent a branch namespace points at when it sits outside the string hierarchy. A branch deploy that suppresses a coverage backfill now says so in the deploy report instead of leaving the author to work out why nothing ran. --- .../internal/deployment/orchestrator.py | 16 ++- .../internal/namespaces.py | 32 +++--- .../tests/api/namespaces_test.py | 48 ++++++++ .../internal/deployment/orchestration_test.py | 12 +- .../tests/internal/git/test_validation.py | 108 ++++++++++++++++++ 5 files changed, 195 insertions(+), 21 deletions(-) diff --git a/datajunction-server/datajunction_server/internal/deployment/orchestrator.py b/datajunction-server/datajunction_server/internal/deployment/orchestrator.py index 4a9d6c364..0e216c786 100644 --- a/datajunction-server/datajunction_server/internal/deployment/orchestrator.py +++ b/datajunction-server/datajunction_server/internal/deployment/orchestrator.py @@ -2696,11 +2696,23 @@ async def _plan_coverage_backfill( The span is recorded beside the cube's other backfills and never asked for twice, since availability does not move until the backfill lands. A branch deploy asks for nothing: it previews what the push would give its author, - and a preview does not spend hundreds of partition runs. + and a preview does not spend hundreds of partition runs. It says so in + the report, so the missing backfill reads as a choice. """ coverage = block.coverage partition = coverage_partition(revision) - if not coverage or partition is None or await self._is_branch_deploy(): + if not coverage or partition is None: + return + if await self._is_branch_deploy(): + self.deployed_results.append( + DeploymentResult( + name=revision.name, + deploy_type=DeploymentResult.Type.MATERIALIZATION, + status=DeploymentResult.Status.SKIPPED, + operation=DeploymentResult.Operation.NOOP, + message="no coverage backfill on a branch deploy", + ), + ) return span = coverage.span(datetime.now(UTC).date()) if span is None: diff --git a/datajunction-server/datajunction_server/internal/namespaces.py b/datajunction-server/datajunction_server/internal/namespaces.py index 24c25ce29..9a884264c 100644 --- a/datajunction-server/datajunction_server/internal/namespaces.py +++ b/datajunction-server/datajunction_server/internal/namespaces.py @@ -286,35 +286,29 @@ def resolve_git_info_from_map( None, ) - # Resolve config_ns: find the git root (has github_repo_path). - # If branch_ns.parent_namespace points outside the string hierarchy (a sibling), - # use the FK-hop parent if it was pre-loaded into ns_map. - # Otherwise, the git root is reachable via string ancestors. - config_ns: NodeNamespace | None = None + # Rows git config resolves from, most specific first. + # A pre-loaded FK parent outside the hierarchy leads. + candidates: list[NodeNamespace] = [] if ( branch_ns and branch_ns.parent_namespace and branch_ns.parent_namespace not in ancestor_names and branch_ns.parent_namespace in ns_map ): - fk_parent = ns_map[branch_ns.parent_namespace] - if fk_parent.github_repo_path: - config_ns = fk_parent - if not config_ns: - config_ns = next( - ( - ns_map[n] - for n in reversed_names - if ns_map.get(n) and ns_map[n].github_repo_path - ), - None, - ) + candidates.append(ns_map[branch_ns.parent_namespace]) + candidates.extend(ns_map[n] for n in reversed_names if ns_map.get(n)) + + config_ns = next((ns for ns in candidates if ns.github_repo_path), None) if not config_ns: return None branch = branch_ns.git_branch if branch_ns else None - default_branch = config_ns.default_branch + # The default branch may sit on another row. + default_branch = next( + (ns.default_branch for ns in candidates if ns.default_branch), + None, + ) # Effective git_only cascades: any ancestor (or the namespace itself) # with git_only=True locks all descendants. Without this the UI would # treat a child namespace as editable when its parent is locked, @@ -363,6 +357,8 @@ async def get_git_info_for_namespace( load the git root (which carries ``github_repo_path`` / ``git_path``). Otherwise, look for ``github_repo_path`` among the string ancestors (self-contained root case). + 3. Take ``default_branch`` from the first row that has one, in the same + order — branch namespaces carry the repo path but not the default branch. """ ancestor_names = get_parent_namespaces(namespace) + [namespace] stmt = select(NodeNamespace).where(NodeNamespace.namespace.in_(ancestor_names)) diff --git a/datajunction-server/tests/api/namespaces_test.py b/datajunction-server/tests/api/namespaces_test.py index dcf67ff96..36e8d8dde 100644 --- a/datajunction-server/tests/api/namespaces_test.py +++ b/datajunction-server/tests/api/namespaces_test.py @@ -3690,6 +3690,54 @@ async def test_hard_delete_git_default_branch_namespace_blocked( assert "default branch" in response.json()["message"] +@pytest.mark.asyncio +async def test_delete_default_branch_owning_repo_blocked( + module__client_with_all_examples: AsyncClient, +) -> None: + """ + A branch namespace that carries its own ``github_repo_path`` -- the shape + branch creation writes -- still resolves its repo's default branch from the + git root, so both deletes are refused. + """ + root = "ownrepo.root" + branch_ns = "ownrepo.root.main" + + await module__client_with_all_examples.post(f"/namespaces/{root}/") + await module__client_with_all_examples.post(f"/namespaces/{branch_ns}/") + await module__client_with_all_examples.patch( + f"/namespaces/{root}/git", + json={"github_repo_path": "corp/ownrepo", "default_branch": "main"}, + ) + + # Branch creation copies the repo path onto the branch namespace. + await module__client_with_all_examples.patch( + f"/namespaces/{branch_ns}/git", + json={"github_repo_path": "corp/ownrepo"}, + ) + await module__client_with_all_examples.patch( + f"/namespaces/{branch_ns}/git", + json={"parent_namespace": root, "git_branch": "main"}, + ) + + refusal = ( + f"Cannot delete namespace `{branch_ns}`: it is the default branch " + "of a git-backed namespace (corp/ownrepo). " + "Only non-default branch namespaces can be deleted." + ) + + response = await module__client_with_all_examples.delete( + f"/namespaces/{branch_ns}/", + ) + assert response.status_code == 422 + assert response.json()["message"] == refusal + + response = await module__client_with_all_examples.delete( + f"/namespaces/{branch_ns}/hard/", + ) + assert response.status_code == 422 + assert response.json()["message"] == refusal + + @pytest.mark.asyncio async def test_delete_non_default_git_branch_namespace_allowed( module__client_with_all_examples: AsyncClient, diff --git a/datajunction-server/tests/internal/deployment/orchestration_test.py b/datajunction-server/tests/internal/deployment/orchestration_test.py index b18938708..1d6b95ee0 100644 --- a/datajunction-server/tests/internal/deployment/orchestration_test.py +++ b/datajunction-server/tests/internal/deployment/orchestration_test.py @@ -4184,7 +4184,8 @@ async def test_a_branch_deploy_asks_for_nothing( ): """ A branch namespace previews what the push would give its author, and a - preview does not spend hundreds of partition runs. + preview does not spend hundreds of partition runs. The report says the + backfill was skipped, so the author is not left guessing. """ revision, materialization = await self._cube( session, @@ -4203,6 +4204,15 @@ async def test_a_branch_deploy_asks_for_nothing( assert self._queued(orchestrator) == [] assert await self._recorded(session, materialization) == [] + assert orchestrator.deployed_results == [ + DeploymentResult( + name="default.a_cube", + deploy_type=DeploymentResult.Type.MATERIALIZATION, + status=DeploymentResult.Status.SKIPPED, + operation=DeploymentResult.Operation.NOOP, + message="no coverage backfill on a branch deploy", + ), + ] @pytest.mark.asyncio async def test_an_uncountable_window_is_warned_about( diff --git a/datajunction-server/tests/internal/git/test_validation.py b/datajunction-server/tests/internal/git/test_validation.py index d12bbe60e..5ccfb5068 100644 --- a/datajunction-server/tests/internal/git/test_validation.py +++ b/datajunction-server/tests/internal/git/test_validation.py @@ -828,6 +828,114 @@ async def test_is_default_branch_false_when_no_default_branch( assert result is not None assert result["is_default_branch"] is False + @pytest.mark.asyncio + async def test_default_branch_from_ancestor_row(self, session: AsyncSession): + """The branch namespace owns the repo path; its parent owns default_branch.""" + session.add( + NodeNamespace( + namespace="demo.metrics", + github_repo_path="corp/dj-nodes-example", + default_branch="main", + git_path="defs/", + ), + ) + session.add( + NodeNamespace( + namespace="demo.metrics.main", + github_repo_path="corp/dj-nodes-example", + git_branch="main", + git_path="defs/", + parent_namespace="demo.metrics", + ), + ) + await session.commit() + + result = await get_git_info_for_namespace(session, "demo.metrics.main") + + assert result == { + "repo": "corp/dj-nodes-example", + "branch": "main", + "default_branch": "main", + "path": "defs/", + "is_default_branch": True, + "parent_namespace": "demo.metrics", + "git_only": False, + "git_root_namespace": "demo.metrics.main", + "branch_namespace": "demo.metrics.main", + } + + @pytest.mark.asyncio + async def test_feature_branch_with_own_repo(self, session: AsyncSession): + """A feature branch owning the repo path is not the default branch.""" + session.add( + NodeNamespace( + namespace="demo.metrics", + github_repo_path="corp/dj-nodes-example", + default_branch="main", + git_path="defs/", + ), + ) + session.add( + NodeNamespace( + namespace="demo.metrics.test_cube", + github_repo_path="corp/dj-nodes-example", + git_branch="test_cube", + git_path="defs/", + parent_namespace="demo.metrics", + ), + ) + await session.commit() + + result = await get_git_info_for_namespace(session, "demo.metrics.test_cube") + + assert result == { + "repo": "corp/dj-nodes-example", + "branch": "test_cube", + "default_branch": "main", + "path": "defs/", + "is_default_branch": False, + "parent_namespace": "demo.metrics", + "git_only": False, + "git_root_namespace": "demo.metrics.test_cube", + "branch_namespace": "demo.metrics.test_cube", + } + + @pytest.mark.asyncio + async def test_default_branch_via_fk_parent(self, session: AsyncSession): + """The git root is reached by the FK hop, not by name.""" + session.add( + NodeNamespace( + namespace="roots.repo", + github_repo_path="org/repo", + default_branch="main", + git_path="defs/", + ), + ) + session.add( + NodeNamespace( + namespace="branches.main", + github_repo_path="org/repo", + git_branch="main", + git_path="defs/", + parent_namespace="roots.repo", + ), + ) + await session.commit() + + result = await get_git_info_for_namespace(session, "branches.main") + + assert result == { + "repo": "org/repo", + "branch": "main", + "default_branch": "main", + "path": "defs/", + "is_default_branch": True, + "parent_namespace": "roots.repo", + "git_only": False, + "git_root_namespace": "roots.repo", + "branch_namespace": "branches.main", + } + # ------------------------------------------------------------------ # parent_namespace # ------------------------------------------------------------------ From 3554faac3857a8568a1cd6f35036c66011130feb Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Mon, 24 Aug 2026 04:00:30 -0700 Subject: [PATCH 2/3] Use neutral names in the git resolution tests --- .../tests/internal/git/test_validation.py | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/datajunction-server/tests/internal/git/test_validation.py b/datajunction-server/tests/internal/git/test_validation.py index 5ccfb5068..976efdecc 100644 --- a/datajunction-server/tests/internal/git/test_validation.py +++ b/datajunction-server/tests/internal/git/test_validation.py @@ -833,35 +833,35 @@ async def test_default_branch_from_ancestor_row(self, session: AsyncSession): """The branch namespace owns the repo path; its parent owns default_branch.""" session.add( NodeNamespace( - namespace="demo.metrics", - github_repo_path="corp/dj-nodes-example", + namespace="shop.metrics", + github_repo_path="corp/examplerepo", default_branch="main", git_path="defs/", ), ) session.add( NodeNamespace( - namespace="demo.metrics.main", - github_repo_path="corp/dj-nodes-example", + namespace="shop.metrics.main", + github_repo_path="corp/examplerepo", git_branch="main", git_path="defs/", - parent_namespace="demo.metrics", + parent_namespace="shop.metrics", ), ) await session.commit() - result = await get_git_info_for_namespace(session, "demo.metrics.main") + result = await get_git_info_for_namespace(session, "shop.metrics.main") assert result == { - "repo": "corp/dj-nodes-example", + "repo": "corp/examplerepo", "branch": "main", "default_branch": "main", "path": "defs/", "is_default_branch": True, - "parent_namespace": "demo.metrics", + "parent_namespace": "shop.metrics", "git_only": False, - "git_root_namespace": "demo.metrics.main", - "branch_namespace": "demo.metrics.main", + "git_root_namespace": "shop.metrics.main", + "branch_namespace": "shop.metrics.main", } @pytest.mark.asyncio @@ -869,35 +869,35 @@ async def test_feature_branch_with_own_repo(self, session: AsyncSession): """A feature branch owning the repo path is not the default branch.""" session.add( NodeNamespace( - namespace="demo.metrics", - github_repo_path="corp/dj-nodes-example", + namespace="shop.metrics", + github_repo_path="corp/examplerepo", default_branch="main", git_path="defs/", ), ) session.add( NodeNamespace( - namespace="demo.metrics.test_cube", - github_repo_path="corp/dj-nodes-example", + namespace="shop.metrics.featureone", + github_repo_path="corp/examplerepo", git_branch="test_cube", git_path="defs/", - parent_namespace="demo.metrics", + parent_namespace="shop.metrics", ), ) await session.commit() - result = await get_git_info_for_namespace(session, "demo.metrics.test_cube") + result = await get_git_info_for_namespace(session, "shop.metrics.featureone") assert result == { - "repo": "corp/dj-nodes-example", + "repo": "corp/examplerepo", "branch": "test_cube", "default_branch": "main", "path": "defs/", "is_default_branch": False, - "parent_namespace": "demo.metrics", + "parent_namespace": "shop.metrics", "git_only": False, - "git_root_namespace": "demo.metrics.test_cube", - "branch_namespace": "demo.metrics.test_cube", + "git_root_namespace": "shop.metrics.featureone", + "branch_namespace": "shop.metrics.featureone", } @pytest.mark.asyncio From 4a59f53b8691a7f37520b3b247d74db737795e80 Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Sat, 19 Sep 2026 00:38:08 -0700 Subject: [PATCH 3/3] Require the namespace itself to be the git root for is_default_branch is_default_branch treated any namespace with no git_branch anywhere in its ancestor chain as the default branch, even when that namespace was just a plain child sitting under a git root (e.g. a branch namespace created without git_branch ever set on its own row). That falsely marked such children as the protected default-branch namespace, making them permanently undeletable. Now the "no branch" case only counts when the namespace resolves as its own git root. --- .../internal/namespaces.py | 4 ++- .../tests/internal/git/test_validation.py | 25 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/datajunction-server/datajunction_server/internal/namespaces.py b/datajunction-server/datajunction_server/internal/namespaces.py index 9a884264c..e7dc771ff 100644 --- a/datajunction-server/datajunction_server/internal/namespaces.py +++ b/datajunction-server/datajunction_server/internal/namespaces.py @@ -323,7 +323,9 @@ def resolve_git_info_from_map( "default_branch": default_branch, "path": config_ns.git_path, "is_default_branch": ( - branch is None # root namespace — no branch means it IS the default + # No branch means default only when this namespace IS the git + # root itself — not merely when no ancestor has git_branch set. + (branch is None and config_ns.namespace == namespace) or (default_branch is not None and branch == default_branch) ), "parent_namespace": branch_ns.parent_namespace if branch_ns else None, diff --git a/datajunction-server/tests/internal/git/test_validation.py b/datajunction-server/tests/internal/git/test_validation.py index 976efdecc..1e053e22b 100644 --- a/datajunction-server/tests/internal/git/test_validation.py +++ b/datajunction-server/tests/internal/git/test_validation.py @@ -801,6 +801,31 @@ async def test_is_default_branch_true_when_no_git_branch( assert result["branch"] is None assert result["is_default_branch"] is True + @pytest.mark.asyncio + async def test_is_default_branch_false_for_plain_child_of_git_root( + self, + session: AsyncSession, + ): + """A plain child namespace (no git_branch of its own) under a git root is + NOT the default branch — only the git root itself gets that treatment when + no git_branch is found anywhere in the ancestor chain.""" + session.add( + NodeNamespace( + namespace="proj", + github_repo_path="org/repo", + default_branch="main", + ), + ) + session.add(NodeNamespace(namespace="proj.stale_child")) + await session.commit() + + result = await get_git_info_for_namespace(session, "proj.stale_child") + + assert result is not None + assert result["branch"] is None + assert result["git_root_namespace"] == "proj" + assert result["is_default_branch"] is False + @pytest.mark.asyncio async def test_is_default_branch_false_when_no_default_branch( self,