nastra commented on code in PR #6837:
URL: https://github.com/apache/iceberg/pull/6837#discussion_r1108267839
##########
aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java:
##########
@@ -105,6 +106,14 @@ public String token() {
return properties().get(OAuth2Properties.TOKEN);
}
+ @Value.Lazy
+ boolean refreshAuthByDefault() {
+ return PropertyUtil.propertyAsBoolean(
+ properties(),
+ CatalogProperties.AUTH_DEFAULT_REFRESH_ENABLED,
+ CatalogProperties.AUTH_DEFAULT_REFRESH_ENABLED_DEFAULT);
Review Comment:
I would like to encourage reviewers here to focus on what we can do to make
things easier to understand & use for users. I understand that this was the
original intent of the property, but the code in this area changed so that the
current behavior of this property is causing more confusion than necessary.
I don't think adding a new property helps (the more configuration choices
you have, the more difficult it gets). Users that will end up using this
property will not understand the difference between this property and a new
property.
Not to blame anyone, but experience shows that most users (including myself)
will read documentation as a last resort. They will use the first property they
find that implies from its name it does X. Adding new properties here is just
making things more difficult.
As I tried to outline in the PR description I believe it is better to use
`AUTH_DEFAULT_REFRESH_ENABLED` to fully control the token refresh behavior to
not cause any confusion for users. This however implies that we change its
default from `false` to `true`.
It would be great to get thoughts and opinions from other folks here as
well: @amogh-jahagirdar @jackye1995 @bryanck @danielcweeks
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
