support reading "expires_in" when the API passes the value as string #23921
Conversation
There was a problem hiding this comment.
@leo-schick thanks for your contribution! We actually just received this same contribution yesterday here: https://github.com/airbytehq/airbyte/pull/20301/files and it will be released very soon. So you should be able to receive the fix today
|
@leo-schick actually come to think of it, I see you have one additional bugfix not included in that PR. I've just merged that PR to master. Could you pull master into your branch, add a test case for the extra file you modified not modified in that PR, and I'll merge your contribution as well? |
|
@leo-schick please pull master and lmk when it's ready for a review |
leo-schick
force-pushed
the
fix-oauth-stringed-expires-in
branch
from
6f027ec to
a8045a4
Compare
|
@sherifnada done |
| @@ -95,7 +95,7 @@ def refresh_access_token(self) -> Tuple[str, int]: | |||
| ) | |||
| response.raise_for_status() | |||
| response_json = response.json() | |||
| return response_json["access_token"], response_json["expires_in"] | |||
| return response_json["access_token"], int(response_json["expires_in"]) | |||
There was a problem hiding this comment.
@leo-schick could you add a unit test for this case?
There was a problem hiding this comment.
@sherifnada I extended the existing unit tests with some assert checks
…irbytehq#23921) Co-authored-by: Sherif A. Nada <snadalive@gmail.com>
…irbytehq#23921) Co-authored-by: Sherif A. Nada <snadalive@gmail.com>
…23921) Co-authored-by: Sherif A. Nada <snadalive@gmail.com>
…23921) Co-authored-by: Sherif A. Nada <snadalive@gmail.com>
What
I was dealing with the OAuth2 implementation of trustpilot.com and they pass the
expires_inOAuth parameter as string and not as integer.This results then in the following error:
With this PR, we would cast the value from
expires_ininto an integer.How
using
int(..)around the value. This then supports both use cases: string and integer values🚨 User Impact 🚨
No breaking changes.