lishuxu commented on code in PR #985:
URL: https://github.com/apache/iceberg-cpp/pull/985#discussion_r4323539182
##########
src/iceberg/catalog/rest/auth/auth_manager.cc:
##########
@@ -217,21 +243,31 @@ class OAuth2Manager : public AuthManager {
Result<std::shared_ptr<AuthSession>> ContextualSession(
const SessionContext& context, std::shared_ptr<AuthSession> parent)
override {
- // TODO(lishuxu): Add child-session caching and refresh, matching Java
- // AuthSessionCache.
+ // Use session_id as cache key for contextual sessions
+ std::string cache_key = context.session_id.empty() ? "" : "ctx:" +
context.session_id;
return MaybeCreateChildSession(context.credentials,
/*allow_credential=*/true,
- std::move(parent));
+ std::move(parent), cache_key);
}
Result<std::shared_ptr<AuthSession>> TableSession(
[[maybe_unused]] const TableIdentifier& table,
const std::unordered_map<std::string, std::string>& properties,
std::shared_ptr<AuthSession> parent) override {
+ // Use token value as cache key for table sessions
+ auto token_it = properties.find(AuthProperties::kToken.key());
+ std::string cache_key;
+ if (token_it != properties.end() && !token_it->second.empty()) {
+ cache_key = "tbl:" + token_it->second;
Review Comment:
Good catch, the child credential is now selected once, and the table cache
key is the value of the token actually used, plain or typed (matching Java's
properties::get). Added tests for both.
--
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]
