Add AsyncMappingWatcher and newChildAsync#1272
Conversation
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request introduces asynchronous mapping capabilities to the Watcher API. The Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
client/java-armeria/src/test/java/com/linecorp/centraldogma/client/armeria/WatcherTest.java (1)
125-154: Please add a reverse-completion regression test.These tests cover exceptional paths, but the fragile part of
AsyncMappingWatcheris the out-of-order success path. A deterministic test that completes revision N+1 before revision N would lock down theisUpdate(...)/AtomicReferencelogic this PR is introducing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/java-armeria/src/test/java/com/linecorp/centraldogma/client/armeria/WatcherTest.java` around lines 125 - 154, Add a deterministic regression test that exercises AsyncMappingWatcher when an async mapping for revision N+1 completes before revision N to validate the isUpdate/AtomicReference logic: create a watcher (like in mapperAsyncFailure), capture its initialValueFuture(), then create two CompletableFutures for successive revisions and feed them to the watcher via newChildAsync so you can complete the N+1 future before completing N, then assert the watcher resolves to the N+1 result (use watcher.initialValueFuture().join() or appropriate assertion) and close the original watcher; reference AsyncMappingWatcher behavior by using newChildAsync, isUpdate semantics and AtomicReference expectations in the test name (e.g., mapperAsyncReverseCompletion) so the test deterministically verifies out-of-order success handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@client/java/src/main/java/com/linecorp/centraldogma/client/AsyncMappingWatcher.java`:
- Around line 94-101: The mapper.apply(value) call in AsyncMappingWatcher can
return null which causes a NPE at mappedValueFuture.whenComplete and bypasses
the failure handling; update the code after calling mapper.apply(value) (the
mappedValueFuture variable) to check for null and if null call
reportFailure.accept(new NullPointerException("mapper returned null")) and also
completeExceptionally(initialValueFuture, sameException) or otherwise ensure
initialValueFuture is completed exceptionally, then return so execution does not
reach mappedValueFuture.whenComplete; reference symbols: AsyncMappingWatcher,
mapper.apply, mappedValueFuture, reportFailure, initialValueFuture,
whenComplete.
- Around line 101-118: The whenComplete handler for mappedValueFuture must
ignore completions after the watcher has been closed: inside the
mappedValueFuture.whenComplete((mappedValue, e) -> { ... }) lambda, check the
watcher shutdown flag (e.g. closed or a similar AtomicBoolean) at the top and
return immediately if closed to avoid calling reportFailure.accept(e), updating
mappedLatest, notifyListeners or completing initialValueFuture for in-flight
futures; keep the existing failure handling only when not closed so late
completions cannot overwrite mappedLatest or trigger listeners on a closed
AsyncMappingWatcher.
---
Nitpick comments:
In
`@client/java-armeria/src/test/java/com/linecorp/centraldogma/client/armeria/WatcherTest.java`:
- Around line 125-154: Add a deterministic regression test that exercises
AsyncMappingWatcher when an async mapping for revision N+1 completes before
revision N to validate the isUpdate/AtomicReference logic: create a watcher
(like in mapperAsyncFailure), capture its initialValueFuture(), then create two
CompletableFutures for successive revisions and feed them to the watcher via
newChildAsync so you can complete the N+1 future before completing N, then
assert the watcher resolves to the N+1 result (use
watcher.initialValueFuture().join() or appropriate assertion) and close the
original watcher; reference AsyncMappingWatcher behavior by using newChildAsync,
isUpdate semantics and AtomicReference expectations in the test name (e.g.,
mapperAsyncReverseCompletion) so the test deterministically verifies
out-of-order success handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2a9873af-996f-4a71-ade0-8d8c2362e190
📒 Files selected for processing (3)
client/java-armeria/src/test/java/com/linecorp/centraldogma/client/armeria/WatcherTest.javaclient/java/src/main/java/com/linecorp/centraldogma/client/AsyncMappingWatcher.javaclient/java/src/main/java/com/linecorp/centraldogma/client/Watcher.java
| final CompletableFuture<? extends U> mappedValueFuture; | ||
| try { | ||
| mappedValueFuture = mapper.apply(value); | ||
| } catch (Exception e) { | ||
| reportFailure.accept(e); | ||
| return; | ||
| } | ||
| mappedValueFuture.whenComplete((mappedValue, e) -> { |
There was a problem hiding this comment.
Reject null futures from the mapper.
If mapper.apply(value) returns null, Line 101 throws outside the current failure path, so the child watcher can fail without going through reportFailure(...) or completing initialValueFuture consistently.
Suggested fix
final CompletableFuture<? extends U> mappedValueFuture;
try {
- mappedValueFuture = mapper.apply(value);
+ mappedValueFuture = requireNonNull(mapper.apply(value),
+ "mapper.apply() returned null CompletableFuture");
} catch (Exception e) {
reportFailure.accept(e);
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final CompletableFuture<? extends U> mappedValueFuture; | |
| try { | |
| mappedValueFuture = mapper.apply(value); | |
| } catch (Exception e) { | |
| reportFailure.accept(e); | |
| return; | |
| } | |
| mappedValueFuture.whenComplete((mappedValue, e) -> { | |
| final CompletableFuture<? extends U> mappedValueFuture; | |
| try { | |
| mappedValueFuture = requireNonNull(mapper.apply(value), | |
| "mapper.apply() returned null CompletableFuture"); | |
| } catch (Exception e) { | |
| reportFailure.accept(e); | |
| return; | |
| } | |
| mappedValueFuture.whenComplete((mappedValue, e) -> { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@client/java/src/main/java/com/linecorp/centraldogma/client/AsyncMappingWatcher.java`
around lines 94 - 101, The mapper.apply(value) call in AsyncMappingWatcher can
return null which causes a NPE at mappedValueFuture.whenComplete and bypasses
the failure handling; update the code after calling mapper.apply(value) (the
mappedValueFuture variable) to check for null and if null call
reportFailure.accept(new NullPointerException("mapper returned null")) and also
completeExceptionally(initialValueFuture, sameException) or otherwise ensure
initialValueFuture is completed exceptionally, then return so execution does not
reach mappedValueFuture.whenComplete; reference symbols: AsyncMappingWatcher,
mapper.apply, mappedValueFuture, reportFailure, initialValueFuture,
whenComplete.
There was a problem hiding this comment.
Mapper cannot actually return null given the function type?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for `@NullMarked` annotations in the relevant package
fd 'package-info.java' --exec grep -l 'NullMarked' {}
# Also check for NullMarked at the class level in the same package
rg '@NullMarked' client/java/src/main/java/com/linecorp/centraldogma/client/ -n
# Check if NullAway or similar is configured
rg 'NullAway\|nullaway\|NullMarked' build.gradle --include='*.gradle' -rn 2>/dev/null || true
rg 'NullAway\|nullaway\|NullMarked' --type=gradle -rn 2>/dev/null || true
Length of output: 6362
@m50d, you are correct. The com.linecorp.centraldogma.client package is annotated with @NullMarked (Line 21 of package-info.java), so under JSpecify semantics the unannotated return type CompletableFuture<? extends U> is implicitly @NonNull. A mapper that returns null already violates the declared contract at the static-analysis level, making the defensive requireNonNull unnecessary. I'll withdraw this suggestion.
client/java/src/main/java/com/linecorp/centraldogma/client/AsyncMappingWatcher.java
Outdated
Show resolved
Hide resolved
|
Thanks for the PR! |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/java/src/main/java/com/linecorp/centraldogma/client/MappingWatcher.java (1)
52-58:⚠️ Potential issue | 🔴 CriticalUse the inherited
closedflag here.After the
AbstractMappingWatcherrefactor, this class still has its ownclosedfield at Line 54.AbstractMappingWatcher.close()updates the base flag, but Line 65 reads the shadow field, so a watcher that has already been closed can still remap parent updates and mutatemappedLatest.Suggested fix
`@Nullable` private volatile Latest<U> mappedLatest; - private volatile boolean closed;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/java/src/main/java/com/linecorp/centraldogma/client/MappingWatcher.java` around lines 52 - 58, Remove the shadowed private volatile boolean closed from MappingWatcher and stop reading that field; use the inherited closed flag from AbstractMappingWatcher instead (replace any checks that reference the local "closed" with the superclass's closed state or its accessor, e.g., super.isClosed() or the inherited protected field). Ensure MappingWatcher only mutates mappedLatest when the inherited closed flag indicates the watcher is open, and verify AbstractMappingWatcher.close() semantics remain relied upon for lifecycle state.
♻️ Duplicate comments (1)
client/java/src/main/java/com/linecorp/centraldogma/client/AsyncMappingWatcher.java (1)
86-93:⚠️ Potential issue | 🟠 MajorReject
nullfutures frommapper.apply(...).If
mapper.apply(value)returnsnull, Line 93 throws beforereportFailureruns, so the child can stay open withinitialValueFuturestill incomplete. Keep the null check inside thetryso it follows the normal failure path.Suggested fix
final CompletableFuture<? extends U> mappedValueFuture; try { - mappedValueFuture = mapper.apply(value); + mappedValueFuture = requireNonNull(mapper.apply(value), + "mapper.apply() returned null CompletableFuture"); } catch (Exception e) { reportFailure.accept(e); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/java/src/main/java/com/linecorp/centraldogma/client/AsyncMappingWatcher.java` around lines 86 - 93, The mapper.apply(...) call in AsyncMappingWatcher currently allows a null CompletableFuture to escape the try/catch, which causes a later NPE and leaves the child watcher in an inconsistent state; wrap the null check inside the same try block that calls mapper.apply(value) (i.e., after assigning mappedValueFuture) and, if mappedValueFuture is null, call reportFailure.accept(new NullPointerException("mapper returned null")) and return so the failure follows the normal error path and initialValueFuture doesn't remain unresolved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@client/java/src/main/java/com/linecorp/centraldogma/client/AbstractMappingWatcher.java`:
- Around line 86-98: The notifyListeners loop checks the closed flag only before
enqueuing, allowing callbacks to run after close and bypasses notifyListener for
same-thread dispatch which skips exception isolation/logging; to fix, move the
closed check inside the dispatched task (or at the start of notifyListener) so
every invocation verifies closed at execution time, and always invoke
notifyListener (even when currentExecutor == executor) via the
executor.execute(...) path or a unified dispatch helper so exceptions are
handled/logged consistently; update references: notifyListeners, notifyListener,
closed, and the watch catch-up invocation to use the same dispatch helper so
post-close and initial catch-up follow identical rules.
In
`@client/java/src/main/java/com/linecorp/centraldogma/client/AsyncMappingWatcher.java`:
- Around line 75-80: The failure handler (reportFailure Consumer) must be gated
by the same staleness rule used in the success path so an out-of-order earlier
failure cannot close newer state; modify the reportFailure logic (and the
equivalent block around lines 93-99) to check the associated
revision/startedRevision against the current published mappedLatest (or a
tracked newestStartedRevision) and only call
initialValueFuture.completeExceptionally(e) and close() when the failure is for
the latest non-stale revision; in short, add the same revision-staleness guard
used when handling successful mapper.apply() completions before performing
failure completion or calling close().
---
Outside diff comments:
In
`@client/java/src/main/java/com/linecorp/centraldogma/client/MappingWatcher.java`:
- Around line 52-58: Remove the shadowed private volatile boolean closed from
MappingWatcher and stop reading that field; use the inherited closed flag from
AbstractMappingWatcher instead (replace any checks that reference the local
"closed" with the superclass's closed state or its accessor, e.g.,
super.isClosed() or the inherited protected field). Ensure MappingWatcher only
mutates mappedLatest when the inherited closed flag indicates the watcher is
open, and verify AbstractMappingWatcher.close() semantics remain relied upon for
lifecycle state.
---
Duplicate comments:
In
`@client/java/src/main/java/com/linecorp/centraldogma/client/AsyncMappingWatcher.java`:
- Around line 86-93: The mapper.apply(...) call in AsyncMappingWatcher currently
allows a null CompletableFuture to escape the try/catch, which causes a later
NPE and leaves the child watcher in an inconsistent state; wrap the null check
inside the same try block that calls mapper.apply(value) (i.e., after assigning
mappedValueFuture) and, if mappedValueFuture is null, call
reportFailure.accept(new NullPointerException("mapper returned null")) and
return so the failure follows the normal error path and initialValueFuture
doesn't remain unresolved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9c4372df-7bd3-4dd0-aba9-901e0c2ababe
📒 Files selected for processing (3)
client/java/src/main/java/com/linecorp/centraldogma/client/AbstractMappingWatcher.javaclient/java/src/main/java/com/linecorp/centraldogma/client/AsyncMappingWatcher.javaclient/java/src/main/java/com/linecorp/centraldogma/client/MappingWatcher.java
| protected final void notifyListeners(Latest<U> latest, @Nullable Executor currentExecutor) { | ||
| if (closed) { | ||
| return; | ||
| } | ||
|
|
||
| for (Map.Entry<BiConsumer<? super Revision, ? super U>, Executor> entry : updateListeners) { | ||
| final BiConsumer<? super Revision, ? super U> listener = entry.getKey(); | ||
| final Executor executor = entry.getValue(); | ||
| if (currentExecutor == executor) { | ||
| notifyListener(latest, listener); | ||
| } else { | ||
| executor.execute(() -> notifyListener(latest, listener)); | ||
| } |
There was a problem hiding this comment.
Centralize callback dispatch so post-close and catch-up notifications follow the same rules.
closed is only checked before the task is enqueued. If close() wins the race after scheduling but before execution, both normal updates and the catch-up callback in watch(...) can still invoke listeners after shutdown. Also, Line 128 bypasses notifyListener(...), so exceptions thrown during the initial catch-up notification are not logged/isolated the same way as later updates.
Suggested refactor
+ private void dispatchListener(Latest<U> latest,
+ BiConsumer<? super Revision, ? super U> listener,
+ Executor executor) {
+ executor.execute(() -> {
+ if (!closed) {
+ notifyListener(latest, listener);
+ }
+ });
+ }
+
protected final void notifyListeners(Latest<U> latest, `@Nullable` Executor currentExecutor) {
if (closed) {
return;
}
for (Map.Entry<BiConsumer<? super Revision, ? super U>, Executor> entry : updateListeners) {
final BiConsumer<? super Revision, ? super U> listener = entry.getKey();
final Executor executor = entry.getValue();
if (currentExecutor == executor) {
- notifyListener(latest, listener);
+ if (!closed) {
+ notifyListener(latest, listener);
+ }
} else {
- executor.execute(() -> notifyListener(latest, listener));
+ dispatchListener(latest, listener, executor);
}
}
}
@@
final Latest<U> mappedLatest = mappedLatest();
if (mappedLatest != null) {
- executor.execute(() -> listener.accept(mappedLatest.revision(), mappedLatest.value()));
+ dispatchListener(mappedLatest, listener, executor);
}
}Also applies to: 119-128
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@client/java/src/main/java/com/linecorp/centraldogma/client/AbstractMappingWatcher.java`
around lines 86 - 98, The notifyListeners loop checks the closed flag only
before enqueuing, allowing callbacks to run after close and bypasses
notifyListener for same-thread dispatch which skips exception isolation/logging;
to fix, move the closed check inside the dispatched task (or at the start of
notifyListener) so every invocation verifies closed at execution time, and
always invoke notifyListener (even when currentExecutor == executor) via the
executor.execute(...) path or a unified dispatch helper so exceptions are
handled/logged consistently; update references: notifyListeners, notifyListener,
closed, and the watch catch-up invocation to use the same dispatch helper so
post-close and initial catch-up follow identical rules.
There was a problem hiding this comment.
This matches the existing MappedWatcher behaviour. I assume the closing is best-effort rather than meant to be 100% reliable.
There was a problem hiding this comment.
@m50d, understood — best-effort close semantics are a reasonable and common design choice for watcher/listener patterns, especially when strict post-close suppression would add synchronization overhead with limited practical benefit. I'll note this for future reviews.
✏️ Learnings added
Learnt from: m50d
Repo: line/centraldogma PR: 1272
File: client/java/src/main/java/com/linecorp/centraldogma/client/AbstractMappingWatcher.java:86-98
Timestamp: 2026-03-09T07:39:08.241Z
Learning: In `AbstractMappingWatcher` (`client/java/src/main/java/com/linecorp/centraldogma/client/AbstractMappingWatcher.java`), the `closed` flag is checked in `notifyListeners` before enqueuing tasks, but not re-checked inside dispatched tasks. This is intentional best-effort close semantics, matching the existing `MappingWatcher` behavior. Do not flag the race between `close()` and listener dispatch as a bug; strict post-close suppression is not a design goal here.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: m50d
Repo: line/centraldogma PR: 1272
File: client/java/src/main/java/com/linecorp/centraldogma/client/AsyncMappingWatcher.java:75-80
Timestamp: 2026-03-09T07:38:15.243Z
Learning: In `AsyncMappingWatcher` (`client/java/src/main/java/com/linecorp/centraldogma/client/AsyncMappingWatcher.java`), closing the watcher on ANY mapper failure — including out-of-order (stale) async failures — is an intentional design decision. Do not flag this as a bug; the intent is to treat any mapper exception as a fatal signal that warrants closing the watcher, even if a newer revision has already succeeded.
| final Consumer<Throwable> reportFailure = (e) -> { | ||
| logger.warn("Unexpected exception is raised from mapper.apply(). mapper: {}", mapper, e); | ||
| if (!initialValueFuture.isDone()) { | ||
| initialValueFuture.completeExceptionally(e); | ||
| } | ||
| close(); |
There was a problem hiding this comment.
Don't let stale async failures close newer state.
The success path suppresses out-of-order completions, but the failure path does not. If a later revision has already published mappedLatest and an earlier future fails afterwards, Line 98 still closes the watcher, so an outdated completion can tear down newer state. Gate failure handling with the same staleness rule, or track the newest started revision separately.
Also applies to: 93-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@client/java/src/main/java/com/linecorp/centraldogma/client/AsyncMappingWatcher.java`
around lines 75 - 80, The failure handler (reportFailure Consumer) must be gated
by the same staleness rule used in the success path so an out-of-order earlier
failure cannot close newer state; modify the reportFailure logic (and the
equivalent block around lines 93-99) to check the associated
revision/startedRevision against the current published mappedLatest (or a
tracked newestStartedRevision) and only call
initialValueFuture.completeExceptionally(e) and close() when the failure is for
the latest non-stale revision; in short, add the same revision-staleness guard
used when handling successful mapper.apply() completions before performing
failure completion or calling close().
There was a problem hiding this comment.
I think this is ok? We intentionally close on any failure, even an out-of-order one.
There was a problem hiding this comment.
@m50d, understood — thanks for the clarification. Closing on any mapper failure (regardless of revision order) is a reasonable defensive posture: if the mapper throws or returns a failed future at all, it signals something is fundamentally wrong with the mapper itself, so tearing down the watcher eagerly makes sense.
Acknowledged and moving on.
✏️ Learnings added
Learnt from: m50d
Repo: line/centraldogma PR: 1272
File: client/java/src/main/java/com/linecorp/centraldogma/client/AsyncMappingWatcher.java:75-80
Timestamp: 2026-03-09T07:38:15.243Z
Learning: In `AsyncMappingWatcher` (`client/java/src/main/java/com/linecorp/centraldogma/client/AsyncMappingWatcher.java`), closing the watcher on ANY mapper failure — including out-of-order (stale) async failures — is an intentional design decision. Do not flag this as a bug; the intent is to treat any mapper exception as a fatal signal that warrants closing the watcher, even if a newer revision has already succeeded.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
jrhee17
left a comment
jrhee17
left a comment
There was a problem hiding this comment.
Left some minor questions, but changes look reasonable to me 👍
client/java/src/main/java/com/linecorp/centraldogma/client/AsyncMappingWatcher.java
Outdated
Show resolved
Hide resolved
client/java/src/main/java/com/linecorp/centraldogma/client/AsyncMappingWatcher.java
Outdated
Show resolved
Hide resolved
client/java/src/main/java/com/linecorp/centraldogma/client/AsyncMappingWatcher.java
Outdated
Show resolved
Hide resolved
client/java/src/main/java/com/linecorp/centraldogma/client/AsyncMappingWatcher.java
Outdated
Show resolved
Hide resolved
client/java-armeria/src/test/java/com/linecorp/centraldogma/client/armeria/WatcherTest.java
Show resolved
Hide resolved
What do you think about keeping only AsyncMappingWatcher and simplifying the implementation by update For example: default <U> Watcher<U> newChild(Function<? super T, ? extends U> mapper, Executor executor) {
requireNonNull(mapper, "mapper");
requireNonNull(executor, "executor");
return AsyncMappingWatcher.of(this, t -> CompletableFuture.supplyAsync(() -> mapper.apply(t), executor),
executor, false);
}
default <U> Watcher<U> newChildAsync(Function<? super T, CompletableFuture<? extends U>> mapper) {
requireNonNull(mapper, "mapper");
return AsyncMappingWatcher.of(this, mapper, null, false);
} |
That's brilliant. 👍 |
It would probably be slightly less efficient, but if we're ok with that then sure, it certainly simplifies things a bit. Let me implement. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@client/java/src/main/java/com/linecorp/centraldogma/client/MappingWatcher.java`:
- Around line 76-84: There are two identical registrations of an exceptionally
handler on parent.initialValueFuture() in MappingWatcher which is a duplicate;
remove one of the duplicate blocks so
parent.initialValueFuture().exceptionally(...) is only called once, leaving the
assignment this.mapper = mapper and a single
initialValueFuture.completeExceptionally(cause) handler intact to preserve
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a63e2fa7-6ec6-4277-8705-902e1e7c0556
📒 Files selected for processing (2)
client/java/src/main/java/com/linecorp/centraldogma/client/MappingWatcher.javaclient/java/src/main/java/com/linecorp/centraldogma/client/Watcher.java
client/java/src/main/java/com/linecorp/centraldogma/client/MappingWatcher.java
Outdated
Show resolved
Hide resolved
client/java/src/main/java/com/linecorp/centraldogma/client/MappingWatcher.java
Outdated
Show resolved
Hide resolved
client/java/src/main/java/com/linecorp/centraldogma/client/MappingWatcher.java
Show resolved
Hide resolved
client/java/src/main/java/com/linecorp/centraldogma/client/MappingWatcher.java
Outdated
Show resolved
Hide resolved
jrhee17
left a comment
jrhee17
left a comment
There was a problem hiding this comment.
Still looks good - CI still seems to be complaining about CLA for some reason
I'm internal, should/can I sign the CLA? |
Yes please - I believe the CLA isn't only about rights/ownership of the work; it's just to document that you’re allowed to contribute, and you’re OK with it being used and redistributed under the project rules |
5 participants
Sometimes we may want to transform a
Watcherwith an asynchronous method. This PR creates an async version ofMappingWatcherand adds a method to be used to create one.AtomicReferenceand slightly more advanced revision-comparison logic to ensure that an earlier update whose transform is slow cannot overwrite a later updateCompletableFuture, it can invoke a custom executor itself without needing any support from our side