nastra commented on code in PR #12562:
URL: https://github.com/apache/iceberg/pull/12562#discussion_r2182392301
##########
core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Manager.java:
##########
@@ -156,19 +156,28 @@ public OAuth2Util.AuthSession tableSession(
@Override
public void close() {
- try {
+ AuthManagerSessionCache<String, OAuth2Util.AuthSession> cache =
sessionCache;
+ this.sessionCache = null;
+ try (cache) {
super.close();
- } finally {
- AuthSessionCache cache = sessionCache;
- this.sessionCache = null;
- if (cache != null) {
- cache.close();
- }
}
}
+ /**
+ * @deprecated since 1.10.0, will be removed in 1.11.0; use {@link
#newAuthSessionCache(String,
+ * Map)}
+ */
+ @Deprecated
protected AuthSessionCache newSessionCache(String managerName, Map<String,
String> properties) {
- return new AuthSessionCache(managerName, sessionTimeout(properties));
Review Comment:
@adutra if you revert the last commit and apply the below diff, wouldn't
that work and avoid all of the extra complexity?
```
--- a/core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Manager.java
+++ b/core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Manager.java
@@ -61,7 +61,7 @@ public class OAuth2Manager extends RefreshingAuthManager {
private RESTClient refreshClient;
private long startTimeMillis;
private OAuthTokenResponse authResponse;
- private AuthSessionCache sessionCache;
+ private SessionCache<String, OAuth2Util.AuthSession> sessionCache;
public OAuth2Manager(String managerName) {
super(managerName + "-token-refresh");
@@ -105,7 +105,7 @@ public class OAuth2Manager extends RefreshingAuthManager
{
RESTClient sharedClient, Map<String, String> properties) {
// This client will be used for token refreshes; it should not have an
auth session.
this.refreshClient = sharedClient.withAuthSession(AuthSession.EMPTY);
- this.sessionCache = newSessionCache(name, properties);
+ this.sessionCache = newAuthSessionCache(name, properties);
AuthConfig config = AuthConfig.fromProperties(properties);
Map<String, String> headers = OAuth2Util.authHeaders(config.token());
OAuth2Util.AuthSession session = new OAuth2Util.AuthSession(headers,
config);
@@ -159,7 +159,7 @@ public class OAuth2Manager extends RefreshingAuthManager
{
try {
super.close();
} finally {
- AuthSessionCache cache = sessionCache;
+ SessionCache<String, OAuth2Util.AuthSession> cache = sessionCache;
this.sessionCache = null;
if (cache != null) {
cache.close();
@@ -175,6 +175,11 @@ public class OAuth2Manager extends
RefreshingAuthManager {
return new AuthSessionCache(managerName, sessionTimeout(properties));
}
+ protected SessionCache<String, OAuth2Util.AuthSession>
newAuthSessionCache(
+ String managerName, Map<String, String> properties) {
+ return new SessionCache<>(managerName, sessionTimeout(properties));
+ }
+
```
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
