jrebmann commented on code in PR #11708:
URL: https://github.com/apache/nifi/pull/11708#discussion_r4102780734
##########
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:
I would prefer to keep this property without a default value for backward
compatibility. An unset or blank value currently disables commit caching, and
introducing a default TTL would change that behavior for existing installations.
Would it be preferable to keep the current opt-in behavior, or should the
cache be enabled by default with an explicit blank value remaining an opt-out?
##########
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:
A regression test was added that populates the commit cache, changes the
remote commit, and verifies that a subsequent write detects the conflict using
the fresh repository state. The test also performs a successful write afterward
and verifies that the commit cache is invalidated and the changed commit is
returned on the next read.
##########
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:
The conflict check now reads the current commit directly from the repository
using getCommitsWithoutCache(...). The cached commit data is still used for
read operations, but it is no longer used as the source of truth for
write-conflict detection.
--
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]
