-
Notifications
You must be signed in to change notification settings - Fork 25
Add foundational branching support to Fluree #1096
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
base: main
Are you sure you want to change the base?
Conversation
970f0e4 to
ce3b475
Compare
9738a09 to
d7891e7
Compare
ce3b475 to
b2a5f9f
Compare
d7891e7 to
42af763
Compare
Preserve storage hash reading functionality from main Add psot index support Update to namespace version ns@v2
…ates, add query validation
… for branch operations
… add publish validation logic
…treamline commit publishing logic
…est using the drop API for main branch deletion
…ity and reuse in deletion and renaming checks
| (recur r (conj addrs (<? (nameservice/publishing-address nsv ledger-alias)))) | ||
| (recur r addrs)) | ||
| (let [published? (<? (nameservice/published-ledger? nsv ledger-alias))] | ||
| (log/info "published-addresses: checking" ledger-alias "published?" published?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should log at the info level.
| (if (<? (nameservice/published-ledger? nsv ledger-alias)) | ||
| (recur r (conj addrs (<? (nameservice/publishing-address nsv ledger-alias)))) | ||
| (recur r addrs)) | ||
| (let [published? (<? (nameservice/published-ledger? nsv ledger-alias))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the nameservice/published-ledger? call would go better in the if condition directly on line 279.
| [ns-record commit-address commit-t] | ||
| "Updates commit address if new t is greater than existing t. | ||
| For branch creation, also validates that branch doesn't already exist." | ||
| [ns-record ledger-alias commit-address commit-t is-branch-creation] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be split into two different functions instead of including the is-branch-creation boolean option here.
| ;; Extract metadata from incoming data | ||
| new-metadata (util.branch/extract-branch-metadata data) | ||
| ;; Check if this is a branch creation (has source metadata) | ||
| is-branch-creation (and (:source-branch new-metadata) (:source-commit new-metadata)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is clearer as a separate branch-creation? function
| "branch" new-branch) | ||
| (util.branch/augment-commit-with-metadata metadata)) | ||
|
|
||
| primary-publisher (:primary-publisher conn) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not for this pr as I think we need an explicit way to connect to a nameservice and provide an address for it, but if the primary publisher (or any other publishers for that matter) should be optional, then it shouldn't be tied to the connection. We should be able to use the same connection to create multiple branches, some of which do have publishers configured, and some without.
|
|
||
| (util.branch/branch-creation-response new-branch metadata source-commit-id))))) | ||
|
|
||
| (defn- same-ledger? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This terminology is confusing to me. I had been using ledger-alias as the full name including the branch, and ledger-name as the name without the branch, and ledger as the catch all for all of a ledgers branches treated as a single unit.
The name of this function and the intermediate bindings used within it do not follow that model. I'm fine with changing my internal terminology, but I do think we should pick consistent terms for those concepts and stick to them.
Summary
This PR introduces foundational branching capabilities to Fluree, enabling parallel database development through branch isolation. This establishes the infrastructure for Git-like version control workflows, allowing multiple branches to coexist and evolve independently.
This PR sits on top of #1088 (branch and time travel naming)
This is a prerequisite to #1095 (merge, rebase, reset branch operations)
This is also a prerequisite to #1102 (cuckoo filter for safe index cleanup across branches)
Core Features
1. Branch-Aware API
All Fluree APIs now support branch notation using the
:separator:2. Branch Creation
Create new branches from existing branches:
3. Branch Isolation
Each branch maintains its own:
Changes to one branch don't affect others until explicitly merged (merge operations coming in future PRs).
4. Time Travel with SHA Support
Navigate to specific commits using SHA:
Implementation Details
Architecture Changes
:separator (e.g.,ledger:branch)mainbranch:to avoid conflicts with branch notationKey Components Modified
Testing
Tests added for:
Run tests with:
Breaking Changes
:character (reserved for branch notation)ledger:branchformat where branch is specifiedMigration
mainbranchmainbranchWhat This Enables (Future Work)
This foundational work enables future PRs to add:
Examples
Creating a Feature Branch Workflow
Next PR
The next PR will add merge operations (rebase, reset) to integrate changes between branches, building on this foundational branching support.