nastra commented on code in PR #11707:
URL: https://github.com/apache/iceberg/pull/11707#discussion_r1871695179
##########
core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Util.java:
##########
@@ -409,7 +409,12 @@ public static OAuthTokenResponse
tokenResponseFromJson(JsonNode json) {
.withIssuedTokenType(JsonUtil.getStringOrNull(ISSUED_TOKEN_TYPE,
json));
if (json.has(EXPIRES_IN)) {
- builder.setExpirationInSeconds(JsonUtil.getInt(EXPIRES_IN, json));
+ // Some IdPs like Entra ID return the expiration time as a string
+ try {
+ builder.setExpirationInSeconds(JsonUtil.getInt(EXPIRES_IN, json));
Review Comment:
the issue with this change is that the Spec states that this should be an
integer:
https://github.com/apache/iceberg/blob/4b52dbd89bf583f1bbdbf0b7a73b62404812ee11/open-api/rest-catalog-open-api.yaml#L3787-L3790
I think we should discuss on the DEV ML what we'd like to do in such a case,
because this opens the codebase for having a precedent where we're less strict
about what we're expecting in terms of value types
--
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]
