pvillard31 commented on code in PR #11708:
URL: https://github.com/apache/nifi/pull/11708#discussion_r4098176227
##########
nifi-extension-bundles/nifi-extension-utils/nifi-git-flow-registry/src/test/java/org/apache/nifi/registry/flow/git/AbstractGitFlowRegistryClientTest.java:
##########
@@ -161,10 +164,49 @@ void createBranchUnsupportedThrowsFlowRegistryException()
throws Exception {
assertTrue(repositoryClient.getCreatedBranchCommit().isEmpty());
}
+ @Test
+ void commitsAreCachedWithinTtl() throws Exception {
Review Comment:
Can the existing tests change the remote commit after the cache is populated
and verify both fresh write conflict checks and changed data after invalidation?
##########
nifi-extension-bundles/nifi-extension-utils/nifi-git-flow-registry/src/main/java/org/apache/nifi/registry/flow/git/AbstractGitFlowRegistryClient.java:
##########
@@ -130,6 +135,13 @@ public abstract class AbstractGitFlowRegistryClient
extends AbstractFlowRegistry
.required(true)
.build();
+ public static final PropertyDescriptor COMMIT_CACHE_TTL = new
PropertyDescriptor.Builder()
Review Comment:
Should this property have a short default TTL so upgrading removes the API
multiplier without extra operator configuration?
##########
nifi-extension-bundles/nifi-gitlab-bundle/nifi-gitlab-extensions/src/main/java/org/apache/nifi/gitlab/GitLabFlowRegistryClient.java:
##########
@@ -32,7 +32,8 @@
import java.util.concurrent.TimeUnit;
@Tags({"git", "gitlab", "registry", "flow"})
-@CapabilityDescription("Flow Registry Client that uses the GitLab REST API to
version control flows in a GitLab Project.")
+@CapabilityDescription("Flow Registry Client that uses the GitLab REST API to
version control flows in a GitLab Project."
Review Comment:
Can we add a space before Note so the capability description does not render
as Project.Note?
##########
nifi-extension-bundles/nifi-extension-utils/nifi-git-flow-registry/src/main/java/org/apache/nifi/registry/flow/git/AbstractGitFlowRegistryClient.java:
##########
@@ -413,7 +432,7 @@ public RegisteredFlowSnapshot registerFlowSnapshot(final
FlowRegistryClientConfi
final String expectedVersion = snapshotMetadata.getVersion();
// Get the current version (latest commit SHA) from the repository
- final List<GitCommit> commits = repositoryClient.getCommits(filePath,
branch);
+ final List<GitCommit> commits = getCommitsCached(filePath, branch);
Review Comment:
Should the write conflict check call repositoryClient.getCommits() directly
so a stale cache cannot reject the current version or accept an old version?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
