-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(powerbi): fix access token expiry #8680
fix(powerbi): fix access token expiry #8680
Conversation
Powerbi returns an expiry time for the access token which is currently ignored. This causes all api calls to fail after 60 minutes, which is the timeout returned by PowerBI. This commit fixes the bug by creating a new token once the old one expires.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks overall good, just one comment
@@ -128,7 +130,7 @@ def get_authorization_header(self): | |||
return {Constant.Authorization: self.get_access_token()} | |||
|
|||
def get_access_token(self): | |||
if self.__access_token is not None: | |||
if self.__access_token is not None and self.__access_token_expiry_time > datetime.now(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please put the expiry condition check in a method is_token_expire()
@siddiquebagwan-gslab fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add a test-case, please refer existing powerbi test-case to mock the client library
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@siddiquebagwan-gslab seems that CI is stuck - is there a way to trigger it to run? (I tried to close and reopen the PR but that just removed your approval so now I need you to approve again) |
97645e6
to
52e1ad4
Compare
Hi @elish7lapid |
Powerbi returns an expiry time for the access
token which is currently ignored.
This causes all api calls to fail after 60 minutes, which is the timeout returned by PowerBI.
This commit fixes the bug by creating a new token
once the old one expires.
Checklist