Add device code print to console functionality #15332
Conversation
…se-console-feature
ghost
added
the
| if (deviceCodeConsumer != null) { | ||
| deviceCodeInfoConsumer = deviceCodeConsumer; | ||
| } else { | ||
| deviceCodeInfoConsumer = deviceCodeInfo -> System.out.println(deviceCodeInfo.getMessage()); | ||
| } |
There was a problem hiding this comment.
Since the consumer is set on the DeviceCodeCredentialBuilder it would be good to have defaults also set in the builder. That's the general pattern we are following for other configurable options like httpclient, retry policies etc. So, we have one place for all defaults.
There was a problem hiding this comment.
That approach comes with the drawback of the configured default living in the memory until the credential object is garbage collected.
This approach will ensure that the default value gets created and garbage collected after the method call finishes.
Given that in memory token cache will be active in our auth policies too, so invocation of this method will not be frequent as well.
There was a problem hiding this comment.
This consumer can be a static constant. I don't think the consumer has to be tied to the lifetime of the credential.
private static final Consumer<DeviceCodeInfo> DEVICE_CODE_CONSUMER = deviceCodeInfo -> System.out.println(deviceCodeInfo.getMessage());
There was a problem hiding this comment.
Yeah, statics are common and from a functionality point of view will work fine.
But, making it static, will prevent it from being garbage collected until the application terminates.
So, from memory point of view, static is worse compared to localizing the variable. In this scenario, the more we localize, the better the situation becomes from memory consumption aspect.
Fixes #13461