Ability to update the repository description #7376
Conversation
|
Would it make sense to add a description update command to |
Exactly, the idea is to use this functionality in the importer, in order to "mark" the repository as fully imported. I have a draft already: #7380 . |
At this point, that is not required as the only update to the repo description will happen internally inside the importer. We can always add that later though. |
| persist, | ||
| (p, retryState) -> { | ||
| try { | ||
| Reference reference = Objects.requireNonNull(persist.fetchReference(REF_REPO.name())); |
There was a problem hiding this comment.
We should probably fix the javadoc of REF_REPO, which currently says Internal reference with always exactly one commit ...
| persist.storeObj(string); | ||
| b.addAdds(commitAdd(KEY_REPO_DESCRIPTION, 0, requireNonNull(string.id()), null, null)); | ||
| b.addAdds( | ||
| commitAdd(KEY_REPO_DESCRIPTION, 0, requireNonNull(string.id()), null, UUID.randomUUID())); |
There was a problem hiding this comment.
This change will only work for new repositories, but existing repositories do not have a content-ID for the repo-description. The update-logic does not work without a content-ID.
I'd prefer to leave the content-ID as null. (There's no need for a CID here)
I know that the repo-config stuff uses a CID, see comment in SLImpl
| existing = existingValueId != null ? fetchString(requireNonNull(existingValueId)) : null; | ||
| } | ||
| } | ||
| if (existingContentId == null) { |
There was a problem hiding this comment.
This makes updating repo-descriptions of already existing repos impossible (those have no CID for repo-description).
Mabe replacing the if with a simple else helps here.
Should also have some comment to explain the logic wrt repo-desc + repo-config
| @Override | ||
| public RepositoryDescription updateRepositoryDescription(RepositoryDescription newDescription) | ||
| throws RetryTimeoutException { | ||
| // prevent modification of read-only attributes |
There was a problem hiding this comment.
This came to my mind after @dimas-b suggestion to introduce an option in quarkus-cli to allow end users to modify the repo description. If we ever do that later, I think we'd need a safety net like this one.
| .putProperties("updated", "true") | ||
| // read-only properties, should be ignored | ||
| .defaultBranchName("main2") |
There was a problem hiding this comment.
Heh, exactly this one might be updated. Let's remove "defaultBranchName" from that check.
I'm not sure whether there'll ever be a command to update arbitrary information - rather per attribute.
| .putProperties("updated", "true") | ||
| // read-only properties, should be ignored | ||
| .defaultBranchName("main2") | ||
| .oldestPossibleCommitTime(Instant.ofEpochSecond(12345)) |
There was a problem hiding this comment.
This and the below setting are actually read-only attributes.
Note: the update repo functionality is implemented in the old model at adapter level, although, it seems unused. This PR brings the same functionality to the new model.