Fix exception handling in AsyncDefaultAdmissionRequestMutator #284
Conversation
| return asyncMutator.mutate(clonedResource, operation) | ||
| .thenApply(resource -> admissionResponseFromMutation(originalResource, resource)) | ||
| .exceptionally(e -> { | ||
| if (e instanceof CompletionException) { |
There was a problem hiding this comment.
This is the actual fix.
| this(mutator, new ObjectMapperCloner<>()); | ||
| } | ||
|
|
||
| public AsyncDefaultAdmissionRequestMutator(AsyncMutator<T> mutator, Cloner<T> cloner) { | ||
| this.mutator = mutator; | ||
| public AsyncDefaultAdmissionRequestMutator(Mutator<T> mutator, Cloner<T> cloner) { |
There was a problem hiding this comment.
This is the internal convenience constructor to enable the use of a Mutator for an async endpoint.
| resource.getMetadata().getLabels().putIfAbsent(MUTATION_TARGET_LABEL, "mutation-test"); | ||
| return resource; | ||
| })); | ||
| return new AsyncAdmissionController<>(new IngressMutator()); |
There was a problem hiding this comment.
This demonstrates the usage of the new convenience constructor. In general this shows how you can de-deduplicate the code for mutators and validators by using old-school inner classes that can be used for sync and async endpoints.
There was a problem hiding this comment.
If you simply run mvn clean install, that should fix it (if remember correctly)
There was a problem hiding this comment.
Hmm, best would be to migrate this repo also to spotless plugin.
There was a problem hiding this comment.
Sadly not, mvn clean install is successful and no files are changed. The Quarkus/JOSDK repos with spotless worked indeed fine for me.
There was a problem hiding this comment.
Ahh, sry, for me neither. Would you like to do the migration to spotless or should I do it (not sure in what timeline) ?
There was a problem hiding this comment.
I'm not a Maven guy (anymore), but I can surely have a look. I'd say this should go in to a new PR and then I rebase this one later?
There was a problem hiding this comment.
yes, that would be ideal, thx!
There was a problem hiding this comment.
I've re-created the branch from main, cherry-picked each commit and ran spotless:apply for all of them. So all commits should be compliant with the code formatting now.
|
Obviously it wasn't enough to import the code style into IDEA. Sadly EDIT: I just |
Signed-off-by: David Sondermann <[email protected]>
Signed-off-by: David Sondermann <[email protected]>
Signed-off-by: David Sondermann <[email protected]>
Donnerbart
force-pushed
the
bugfix/fix-async-mutator-exception-handling
branch
2 times, most recently
from
7ff0dd4 to
d530946
Compare
|
@Donnerbart the build fails, could you pls take a look. Thx |
Signed-off-by: David Sondermann <[email protected]>
…d add constructor for Mutator interface Signed-off-by: David Sondermann <[email protected]>
Donnerbart
force-pushed
the
bugfix/fix-async-mutator-exception-handling
branch
from
d530946 to
6a6492f
Compare
|
It was just a missing import, an oversight when I re-created the branch. |
Fixes #283
AsyncDefaultAdmissionRequestMutatorAsyncAdmissionController(Mutator<T> mutator)constructorMutatorandValidatorclasses inio.javaoperatorsdk.webhook.sample.commons.AdmissionControllersCode Coverage
mainbugfix/fix-async-mutator-exception-handling