You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/05/29 18:42:29 UTC
[GitHub] [iceberg] rdblue opened a new pull request, #4894: Core: Add REST catalog session timeout
rdblue opened a new pull request, #4894:
URL: https://github.com/apache/iceberg/pull/4894
This adds a timeout for REST catalog sessions. After the timeout is reached after a session is used, it will be removed and its token will no longer be refreshed using token exchange.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a diff in pull request #4894: Core: Add REST catalog session timeout
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #4894:
URL: https://github.com/apache/iceberg/pull/4894#discussion_r884313809
##########
core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Properties.java:
##########
@@ -37,8 +37,8 @@ private OAuth2Properties() {
* Interval in milliseconds to wait before attempting to exchange the configured catalog Bearer token.
* By default, token exchange will be attempted after 1 hour.
*/
- public static final String EXCHANGE_TOKEN_MS = "exchange-token-in-ms";
- public static final long EXCHANGE_TOKEN_MS_DEFAULT = 3_600_000; // 1 hour
+ public static final String TOKEN_EXPIRES_IN_MS = "token-expires-in-ms";
Review Comment:
This rename brings the property more in line with the OAuth2 `expires_in` field.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue merged pull request #4894: Core: Add REST catalog session timeout
Posted by GitBox <gi...@apache.org>.
rdblue merged PR #4894:
URL: https://github.com/apache/iceberg/pull/4894
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] danielcweeks commented on a diff in pull request #4894: Core: Add REST catalog session timeout
Posted by GitBox <gi...@apache.org>.
danielcweeks commented on code in PR #4894:
URL: https://github.com/apache/iceberg/pull/4894#discussion_r884314519
##########
core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Properties.java:
##########
@@ -37,8 +37,8 @@ private OAuth2Properties() {
* Interval in milliseconds to wait before attempting to exchange the configured catalog Bearer token.
* By default, token exchange will be attempted after 1 hour.
*/
- public static final String EXCHANGE_TOKEN_MS = "exchange-token-in-ms";
- public static final long EXCHANGE_TOKEN_MS_DEFAULT = 3_600_000; // 1 hour
+ public static final String TOKEN_EXPIRES_IN_MS = "token-expires-in-ms";
Review Comment:
OAuth spec has this as seconds. Should we just use the unit from the spec? I'm not sure milliseconds add much value.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a diff in pull request #4894: Core: Add REST catalog session timeout
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #4894:
URL: https://github.com/apache/iceberg/pull/4894#discussion_r884321221
##########
core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Properties.java:
##########
@@ -37,8 +37,8 @@ private OAuth2Properties() {
* Interval in milliseconds to wait before attempting to exchange the configured catalog Bearer token.
* By default, token exchange will be attempted after 1 hour.
*/
- public static final String EXCHANGE_TOKEN_MS = "exchange-token-in-ms";
- public static final long EXCHANGE_TOKEN_MS_DEFAULT = 3_600_000; // 1 hour
+ public static final String TOKEN_EXPIRES_IN_MS = "token-expires-in-ms";
Review Comment:
All other durations in Iceberg config use milliseconds, so I did this for consistency. I agree it doesn't help much, but I think consistency is important so people don't always need to look up the expected unit.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #4894: Core: Add REST catalog session timeout
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #4894:
URL: https://github.com/apache/iceberg/pull/4894#issuecomment-1140520456
Thanks for reviewing, @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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org