Skip to content
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

CreateOrUpdateLabels* becomes CreateLabels* #64

Closed
wants to merge 1 commit into from
Closed

Conversation

nicksnyder
Copy link
Member

@nicksnyder nicksnyder commented Feb 5, 2024

Our dependency resolution algorithm prefers to select commits by the latest creation date, but that yields non-sensical results if we allow labels to get updated to point to older commits (which the current API permits).

For example, it is currently possible to use CreateOrUpdateLabels to construct a label history like [C1, C2, C1]. Semantically C1 should be the "latest" commit, but since it has a timestamp older than C2, our dependency resolution algorithm would prefer C2.

To avoid this scenario, we should not expose an endpoint to explicitly update the commit a label is pointing to. Instead, the only way to update a label is to use UploadService. This allows users to push the content they want to push (which can be identical content to a previous commit) and we can mint a new commit for them with a current created_at timestamp. In other words, the commit history from the example above would be [C1, C2, C3] where the content of C3 is identical to the content of C1.

The alternative to this is to keep the current API but document it as being an error to attempt to update a label to point to a commit with a created_at timestamp that is earlier than the existing commit the label points to. This alternative is implemented here: #66

@nicksnyder nicksnyder marked this pull request as ready for review February 5, 2024 20:22
@nicksnyder nicksnyder requested a review from bufdev as a code owner February 5, 2024 20:22
@unmultimedio
Copy link
Member

unmultimedio commented Feb 5, 2024

I lean more towards #66 to keep supporting the buf tag command (which probably will evolve to buf label or similar), that adds labels to existing commits w/out necessarily pushing new content.

@nicksnyder
Copy link
Member Author

Fair enough. If everyone is fine with #66 then we can close this PR

@bufdev
Copy link
Member

bufdev commented Feb 5, 2024

I think we should do #66.

@nicksnyder nicksnyder closed this Feb 5, 2024
@nicksnyder nicksnyder deleted the label-create branch February 5, 2024 21:20
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.

3 participants