Skip to content

Fix tracing up tree to handle branches#1852

Merged
shangyian merged 6 commits intoDataJunction:mainfrom
shangyian:better-trace
Mar 10, 2026
Merged

Fix tracing up tree to handle branches#1852
shangyian merged 6 commits intoDataJunction:mainfrom
shangyian:better-trace

Conversation

@shangyian
Copy link
Collaborator

@shangyian shangyian commented Mar 10, 2026

Summary

The old implementation had two traversal strategies that ran somewhat independently: a string hierarchy batch query and an iterative FK chain loop. The FK chain would follow parent_namespace links from any namespace in the chain, even ones without git_branch set, which caused it to incorrectly resolve repo info for partially-configured namespaces.

The new implementation models the actual intended data structure:

  • A branch namespace must have git_branch set. Found by walking string ancestors (nearest/most-specific first).
  • A git root carries github_repo_path/git_path. Found either via one FK hop from branch_ns.parent_namespace (when the root is a sibling, not a string ancestor), or from the string ancestors directly (when root is a string ancestor and no explicit parent_namespace link is needed).

This is at most two queries (one batch + one point lookup), and correctly returns None when no branch_ns exists, even if parent_namespace is set on the namespace. That fixes the case where a namespace has parent_namespace but no git_branch (incomplete config) and was previously leaking through to GitHub auth.

Test Plan

  • PR has an associated issue: #
  • make check passes
  • make test shows 100% unit test coverage

Deployment Plan

@netlify
Copy link

netlify bot commented Mar 10, 2026

Deploy Preview for thriving-cassata-78ae72 canceled.

Name Link
🔨 Latest commit 41f1d92
🔍 Latest deploy log https://app.netlify.com/projects/thriving-cassata-78ae72/deploys/69b06e59636a2e0008dff1d7

@shangyian shangyian marked this pull request as ready for review March 10, 2026 19:57
@shangyian shangyian merged commit d5df593 into DataJunction:main Mar 10, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant