fix: run dynamic call credentials via executor#995
Conversation
|
Caveat: Better to offload the credential fetching when it's non-cached one. Assuming most of the request can reuse a cached the credential, this may pay a small cost for most of the rpcs. |
|
So would it be better to revert the change and just add a hint to the javadocs? Or maybe introduce a new variant that it is explicitly called "async" that takes a private final Function<Executor, CompletableFuture<String>> extraHeadersFutureSupplier;
@Override
public void applyRequestMetadata(final RequestInfo requestInfo, final Executor appExecutor,
final MetadataApplier applier) {
extraHeadersFutureSupplier.apply(executor).whenComplete((header, e) -> {
if (e == null) {
final Metadata extraHeaders = new Metadata();
extraHeaders.put(AUTHORIZATION_HEADER, header);
applier.apply( extraHeaders);
} else {
applier.fail(Status.UNAUTHENTICATED.withCause(e));
}
});
}On the other hand we could just let the user implement that temselves. |
|
Yes, I feel the dynamic credential doesn't provide too much value to the end users. This is an important enough detail (cached/non-cached/async/blocking) which should be exposed to the implementer of the credential. Either way works, but I personally like the ideal to just let the user implement that themselves. |
Fixes #958