You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "danielcweeks (via GitHub)" <gi...@apache.org> on 2023/01/30 22:17:34 UTC

[GitHub] [iceberg] danielcweeks opened a new pull request, #6707: Mark updated auth fields volatile for refresh

danielcweeks opened a new pull request, #6707:
URL: https://github.com/apache/iceberg/pull/6707

   The fields updated in the background during auth refresh should be marked as volatile.


-- 
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 #6707: Mark updated auth fields volatile for refresh

Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #6707:
URL: https://github.com/apache/iceberg/pull/6707#discussion_r1092391292


##########
core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Util.java:
##########
@@ -354,10 +354,10 @@ public static class AuthSession {
     private static int tokenRefreshNumRetries = 5;

Review Comment:
   This is for testing only, so I think it's okay.



-- 
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] bryanck commented on a diff in pull request #6707: Mark updated auth fields volatile for refresh

Posted by "bryanck (via GitHub)" <gi...@apache.org>.
bryanck commented on code in PR #6707:
URL: https://github.com/apache/iceberg/pull/6707#discussion_r1091447817


##########
core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Util.java:
##########
@@ -354,10 +354,10 @@ public static class AuthSession {
     private static int tokenRefreshNumRetries = 5;

Review Comment:
   This could be volatile also, maybe make `refresh()` synchronized as well?



-- 
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 #6707: Mark updated auth fields volatile for refresh

Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue merged PR #6707:
URL: https://github.com/apache/iceberg/pull/6707


-- 
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] bryanck commented on a diff in pull request #6707: Mark updated auth fields volatile for refresh

Posted by "bryanck (via GitHub)" <gi...@apache.org>.
bryanck commented on code in PR #6707:
URL: https://github.com/apache/iceberg/pull/6707#discussion_r1091447817


##########
core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Util.java:
##########
@@ -354,10 +354,10 @@ public static class AuthSession {
     private static int tokenRefreshNumRetries = 5;

Review Comment:
   This could be volatile also. Maybe make `refresh()` below synchronized as well? It should only be called by the schedule thread but might make sense to be on the safe side.



-- 
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