-
Notifications
You must be signed in to change notification settings - Fork 0
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
Revamp CreateCommits to just create Commits and add specialized Push and Rebase endpoints #43
Changes from all commits
b72837d
a301ca1
4ec3aec
6d72e67
cacb418
0ae1cf3
cbc577e
f623f85
b2255fb
988975a
fbf5f23
987d8d8
6881f52
4c7358d
6af255e
ad64ae8
611c669
23d1232
ea3b724
f592c6b
b78f4ca
25c645c
edc5e89
d6538eb
d16c8d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,6 +37,25 @@ service BranchService { | |
rpc ListBranches(ListBranchesRequest) returns (ListBranchesResponse) { | ||
option idempotency_level = NO_SIDE_EFFECTS; | ||
} | ||
// Push existing Commits onto Branches. | ||
// | ||
// This operation is atomic. Either all Commits are pushed to the referenced Branches in the order | ||
// provided or an error is returned. | ||
// | ||
// This operation does not allow the caller to assert the state of Branches before pushing any | ||
// Commits onto them. As a result, multiple concurrent Push operations that target the same Branch | ||
// will all succeed, and the head Commit on the Branch will be determined by the last request that | ||
// succeeded. To deterministically push Commits onto a Branch, use Rebase instead. | ||
rpc Push(PushRequest) returns (PushResponse); | ||
// Rebase existing Commits onto Branches. | ||
// | ||
// This operation is atomic. Either all Branches are rebased as represented in the given | ||
// Operations, or an error is returned. Additionally, each Operation in the request asserts the | ||
// expected head Commit of the branch. If the actual head Commit does not match the expected head | ||
// Commit, all Operations are rejected and an error is returned. | ||
rpc Rebase(RebaseRequest) returns (RebaseResponse) { | ||
option idempotency_level = IDEMPOTENT; | ||
} | ||
} | ||
|
||
message GetBranchesRequest { | ||
|
@@ -98,3 +117,133 @@ message ListBranchesResponse { | |
// The listed Branches. | ||
repeated Branch branches = 2; | ||
} | ||
|
||
message PushRequest { | ||
message Value { | ||
// The Branch to push the Commits to. | ||
// | ||
// If this is a name reference, and the named Branch does not exist, it will be created and the | ||
// given Commits will be the first Commits on the Branch. | ||
BranchRef branch_ref = 1 [(buf.validate.field).required = true]; | ||
// The ID of the Commits to push to the Branch, in the order they should be added. | ||
// | ||
// The last Commit will become the head Commit on the Branch. | ||
// | ||
// If any of these Commits already exist on the Branch, an error is returned. | ||
repeated string commit_ids = 2 [ | ||
(buf.validate.field).repeated.min_items = 1, | ||
(buf.validate.field).repeated.items = { | ||
string: {uuid: true} | ||
} | ||
]; | ||
} | ||
// The Commits to push with the Branches they should be pushed to. | ||
// | ||
// All Values should have unique BranchRefs, that is no two Values should have a BranchRef that | ||
// refers to the same Branch. An error will be returned if any two BranchRefs refer to the same | ||
// Branch. | ||
repeated Value values = 1 [(buf.validate.field).repeated.min_items = 1]; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @saquibmian do we need to support pushing to multiple branches simultaneously - push is last writer wins anyways, so maybe it's not worthwhile? If we got rid of that, we'd be able to simplify this RPC and get rid of the nested There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We do not, but not supporting batching is inconsistent with the rest of the schema. Every RPC today supports batching, except the |
||
|
||
message PushResponse { | ||
// The Branches that were pushed to, in the order they appeared in the request. | ||
repeated Branch branches = 1 [(buf.validate.field).repeated.min_items = 1]; | ||
} | ||
|
||
message RebaseRequest { | ||
// An operation to apply as part of the rebase. | ||
message Operation { | ||
// An operation to create a Branch. | ||
message CreateBranch { | ||
// The Module to create the Branch on. | ||
ModuleRef module_ref = 1 [(buf.validate.field).required = true]; | ||
// The name of the Branch to create. | ||
// | ||
// If a Branch with this name already exists on the Module, this will result in an error. | ||
string branch_name = 2 [ | ||
(buf.validate.field).required = true, | ||
(buf.validate.field).string.max_len = 250 | ||
]; | ||
// The ID of a Commit on another Branch from which to start history. The history of the branch | ||
// will reflect the history as if you called ListCommitHistory for that Branch, starting at | ||
// this Commit. | ||
// | ||
// If this is empty, the created Branch will only have the Commits specified in | ||
// add_commit_ids. | ||
// | ||
// This is primarily designed for use in forking a Branch instead of providing the new | ||
// Branch's entire history. | ||
string base_commit_id = 3 [(buf.validate.field).string.uuid = true]; | ||
// The IDs of the Commits to add to the Branch, in the order they should be added. | ||
// | ||
// If base_commit_id is specified, these will be appended to the Commit history | ||
// that aleady exists up to base_commit_id. | ||
// | ||
// If base_commit_id is not specified, the newly-created Branch will have exactly | ||
// these Commits, in this order. | ||
repeated string add_commit_ids = 4 [(buf.validate.field).repeated.items = { | ||
string: {uuid: true} | ||
}]; | ||
} | ||
|
||
// An operation to update an existing branch. | ||
message UpdateBranch { | ||
// The branch to update. | ||
BranchRef branch_ref = 1 [(buf.validate.field).required = true]; | ||
// The ID of the Commit that the caller expects to be the head Commit on the Branch. | ||
// | ||
// If this ID does not match the ID of the head Commit on the Branch, an error is | ||
// returned. | ||
string expected_head_commit_id = 2 [ | ||
(buf.validate.field).required = true, | ||
(buf.validate.field).string.uuid = true | ||
]; | ||
// The IDs of the Commits to remove from the Branch. | ||
// | ||
// These Commits must appear in order at the end of the Branch's history. Commits are removed | ||
// from history before any Commits from add_commit_ids are added to history. | ||
// | ||
// Note that if all Commits on a Branch's history are specified here and add_commit_ids is | ||
// empty, the resulting Branch's history is empty, and as a result the Branch is deleted. | ||
repeated string remove_commit_ids = 3 [(buf.validate.field).repeated.items = { | ||
string: {uuid: true} | ||
}]; | ||
// The IDs of the Commits to add to the Branch, in the order they should be added. | ||
// | ||
// These are added starting at the head Commit after remove_commit_ids has been applied. | ||
// | ||
// Note that is is valid to have IDs in both add_commit_ids and remove_commit_ids. If | ||
// this is the case, the IDs will be removed from the Branch prior to re-adding them after | ||
// the head Commit via add_commit_ids. | ||
repeated string add_commit_ids = 4 [(buf.validate.field).repeated.items = { | ||
string: {uuid: true} | ||
}]; | ||
} | ||
|
||
oneof value { | ||
option (buf.validate.oneof).required = true; | ||
// Create a new branch. | ||
CreateBranch create_branch = 1; | ||
// Update an existing branch. | ||
UpdateBranch update_branch = 2; | ||
} | ||
} | ||
|
||
// The operations to apply. | ||
// | ||
// The Operations will be applied in the order they are provided. Notably, this means that | ||
// if a CreateBranch Operation precedes a UpdateBranch Operation for the same Branch, the | ||
// UpdateBranch Operation is applied to the newly-created Branch, including any new Commits | ||
// that were added from the CreateBranch Operation. Conversely, if a UpdateBranch Operation | ||
// precedes the CreateBranch operation that creates the Branch, this will result in an error. | ||
repeated Operation operations = 1 [(buf.validate.field).repeated.min_items = 1]; | ||
} | ||
|
||
message RebaseResponse { | ||
// The Branches that were updated. This list will be the unique set of Branches that were updated. | ||
// The length of branches will be less than the length of the input Operations if multiple | ||
// Operations were applied for a single Branch, or if the set of Operations resulted in a Branch's | ||
// deletion. However, the response Branches are ordered in the same order that the Branches first | ||
// appeared in the request Operations. | ||
repeated Branch branches = 1 [(buf.validate.field).repeated.min_items = 1]; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,14 +37,11 @@ service CommitService { | |
} | ||
// Create commits on a Module with associated Content. | ||
// | ||
// This is used by push and sync. | ||
// These commits will not be associated with any Branch or Tag after the completion of | ||
// this operation. To associate created Commits, use BranchService.Push or BranchService.Rebase. | ||
// | ||
// This operation is atomic. Either all Commits and associated content are created or an error is returned. | ||
// | ||
// TODO: PushCommits? Something else? This is creating potentially a bunch of resources. | ||
rpc CreateCommits(CreateCommitsRequest) returns (CreateCommitsResponse) { | ||
option idempotency_level = IDEMPOTENT; | ||
} | ||
rpc CreateCommits(CreateCommitsRequest) returns (CreateCommitsResponse); | ||
// Get the pointers to the content for a given set of Commits, Modules, Branches, Tags, or VCSCommits. | ||
// | ||
// Nodes consist of: | ||
|
@@ -79,9 +76,11 @@ message ResolveCommitsRequest { | |
// - If a Commit is referenced, this just references that specific commit. | ||
// - If a Tag is referenced, this is interpreted to mean the Commit associated with the Tag. | ||
// - If a VCSCommit is referenced, this is interpreted to mean the Commit associated with the VCSCommit. | ||
// - Is a Branch is referenced, this is interpreted to mean the latest Commit on the Branch. | ||
// - Is a Branch is referenced, this is interpreted to mean the head Commit on the Branch. | ||
// - If a Digest is referenced, this is interpreted to mean the latest released Commit that has this Digest. | ||
// Digests referencing unreleased Commits cannot be referenced. | ||
// | ||
// TODO: what is "latest released Commit" if we now have "head Commit"? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need to re-evaluate everywhere in this API we use the word "released" given our new buf sync model Is "released" a property of a BSR commit?
Also need to take a look at branches service and probably rename references to "release branch" to "default branch" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes we need to re-evaluate this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this something we can do separate from this PR, or do we want to formalize "released" now and rope it into this PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should try to land this PR and save this bigger TODO for a followup There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perfect, I'll leave this here for now and we can clean up/formalize "release" after this. |
||
repeated ResourceRef resource_refs = 1 [ | ||
(buf.validate.field).repeated.min_items = 1, | ||
(buf.validate.field).repeated.max_items = 250 | ||
|
@@ -111,9 +110,11 @@ message ListCommitHistoryRequest { | |
// - If a Commit is referenced, history is started at this Commit. | ||
// - If a Tag is referenced, history is started at the Commit associated with the Tag. | ||
// - If a VCSCommit is referenced, history is started at the Commit associated with the VCSCommit. | ||
// - Is a Branch is referenced, history is started at the latest Commit on the Branch. | ||
// - Is a Branch is referenced, history is started at the head Commit on the Branch. | ||
// - If a Digest is referenced, history is started at the latest released Commit that has this Digest. | ||
// Digests referencing unreleased Commits cannot be referenced. | ||
// | ||
// TODO: what is "latest released Commit" if we now have "head Commit"? | ||
ResourceRef resource_ref = 3 [(buf.validate.field).required = true]; | ||
// Only return Commits that have one or more associated Tags. | ||
bool has_tag = 4; | ||
|
@@ -193,41 +194,28 @@ message CreateCommitsRequest { | |
repeated DepNode dep_nodes = 3; | ||
} | ||
|
||
// The pointers to the content for the Modules that should have Commits created for them. | ||
// | ||
// Each ModuleNode must have a unique ModuleRef. | ||
// A commit will be created for each ModuleNode. | ||
repeated ModuleNode module_nodes = 1 [(buf.validate.field).repeated.min_items = 1]; | ||
// Blobs for the FileNodes referenced by module_nodes that are not present on the server. | ||
message Value { | ||
bufdev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// The pointers to the content for the Modules that should have Commits created for them. | ||
// | ||
// Each ModuleNode must have a unique ModuleRef. | ||
// A commit will be created for each ModuleNode. | ||
repeated ModuleNode module_nodes = 1 [(buf.validate.field).repeated.min_items = 1]; | ||
// Associated VCS commit information. | ||
// | ||
// If there are already VCSCommits on the associated Module with a given hash, this | ||
// will result in an error. Otherwise, a new VCSCommit is created. | ||
repeated AssociatedVCSCommit associated_vcs_commits = 2; | ||
} | ||
// The requests to create Commits. | ||
repeated Value values = 1 [(buf.validate.field).repeated.min_items = 1]; | ||
// Blobs for the FileNodes referenced by file_nodes that are not present on the server. | ||
// | ||
// Only Blobs that were returned as missing from GetMissingBlobDigests need to be sent. | ||
// Other Blobs already exist on the server, and will be ignored. | ||
// | ||
// If a FileNode within module_nodes has a Blob that is not on the server, and is not | ||
// If a FileNode has a Blob that is not on the server, and is not | ||
// within missing_blobs, an error will be returned. | ||
repeated buf.registry.storage.v1beta1.Blob missing_blobs = 2; | ||
// The names of Branches that should be associated with this Commit. | ||
// | ||
// If a Branch currently exists on the associated Module with a name, this existing | ||
// Branch will be used. Otherwise, a new Branch will be created for the corresponding name. | ||
// | ||
// If empty, the default branch is assumed as the only branch. | ||
repeated string branch_names = 3 [(buf.validate.field).repeated.items = { | ||
string: {max_len: 250} | ||
}]; | ||
// The names of Tags that should be associated with this Commit. | ||
// | ||
// If a Tag currently exists on the assocated Module with a name, the RPC will error, however | ||
// this will change in the future when we allow Tags to move. If the Tag does not | ||
// currently exist, a new Tag will be created for each name. | ||
repeated string tag_names = 4 [(buf.validate.field).repeated.items = { | ||
string: {max_len: 250} | ||
}]; | ||
// Associated VCS commit information. | ||
// | ||
// If there are already VCSCommits on the associated Module with a given hash, this | ||
// will result in an error. Otherwise, a new VCSCommit is created. | ||
repeated AssociatedVCSCommit associated_vcs_commits = 5; | ||
} | ||
|
||
message CreateCommitsResponse { | ||
|
@@ -250,9 +238,11 @@ message GetCommitNodesRequest { | |
// - If a Commit is referenced, files are returned from this Commit. | ||
// - If a Tag is referenced, files are returned from the Commit associated with the Tag. | ||
// - If a VCSCommit is referenced, files are returned from the Commit associated with the VCSCommit. | ||
// - Is a Branch is referenced, files are returned from the latest Commit on the Branch. | ||
// - Is a Branch is referenced, files are returned from the head Commit on the Branch. | ||
// - If a Digest is referenced, files are returned from the latest released Commit that has this Digest. | ||
// Digests referencing unreleased Commits cannot be referenced. | ||
// | ||
// TODO: what is "latest released Commit" if we now have "head Commit"? | ||
ResourceRef resource_ref = 1 [(buf.validate.field).required = true]; | ||
// Specific file paths to retrieve. | ||
// | ||
|
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 noticed this is required. Does this imply that an empty Branch does not exist, or should we relax this constraint?
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.
Unless this causes a problem elsewhere, it makes sense to me that an empty branch doesn't exist because this is true in Git (a branch pointer has to point to a commit).
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 updated the docs to reflect this.