[3/n][dagster-airbyte] Implement base request method in AirbyteCloudClient #26241
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Dec 3, 2024
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
python_modules/libraries/dagster-airbyte/dagster_airbyte/resources.py
Outdated
Show resolved
Hide resolved
maximearmstrong
force-pushed
the
maxime/rework-airbyte-cloud-2
branch
from
2dc0534 to
9c0212c
Compare
maximearmstrong
force-pushed
the
maxime/rework-airbyte-cloud-3
branch
from
3b10c92 to
b92ce13
Compare
maximearmstrong
force-pushed
the
maxime/rework-airbyte-cloud-2
branch
from
9c0212c to
2930c7f
Compare
maximearmstrong
force-pushed
the
maxime/rework-airbyte-cloud-3
branch
from
b92ce13 to
0ba2ae9
Compare
python_modules/libraries/dagster-airbyte/dagster_airbyte/resources.py
Outdated
Show resolved
Hide resolved
maximearmstrong
force-pushed
the
maxime/rework-airbyte-cloud-2
branch
from
2930c7f to
8645142
Compare
maximearmstrong
force-pushed
the
maxime/rework-airbyte-cloud-3
branch
from
0ba2ae9 to
6b52f15
Compare
maximearmstrong
force-pushed
the
maxime/rework-airbyte-cloud-2
branch
from
8645142 to
f19ad83
Compare
maximearmstrong
force-pushed
the
maxime/rework-airbyte-cloud-3
branch
from
6b52f15 to
a22d971
Compare
maximearmstrong
force-pushed
the
maxime/rework-airbyte-cloud-2
branch
from
f19ad83 to
8397e6a
Compare
maximearmstrong
force-pushed
the
maxime/rework-airbyte-cloud-3
branch
from
a22d971 to
29a997b
Compare
maximearmstrong
changed the title
maximearmstrong
commented
Dec 4, 2024
| @@ -801,35 +805,145 @@ def airbyte_cloud_resource(context) -> AirbyteCloudResource: | |||
|
|
|||
|
|
|||
| @experimental | |||
| class AirbyteCloudClient: | |||
| class AirbyteCloudClient(DagsterModel): | |||
There was a problem hiding this comment.
Using DagsterModel to handle PrivateAttr
maximearmstrong
commented
Dec 4, 2024
| self._access_token_value = str(response["access_token"]) | ||
| self._access_token_timestamp = datetime.now().timestamp() | ||
|
|
||
| def _needs_refreshed_access_token(self) -> bool: |
There was a problem hiding this comment.
The access token must be refreshed every 3 minutes
dpeng817
reviewed
Dec 5, 2024
| """Creates and sends a request to the desired Airbyte REST API endpoint. | ||
|
|
||
| Args: | ||
| method (str): The http method to use for this request (e.g. "POST", "GET", "PATCH"). |
There was a problem hiding this comment.
nit: can probably use the requests enum here?
There was a problem hiding this comment.
The mock response library provides an enum, but the request library doesn't - the request library still uses strings in the implementation.
dpeng817
reviewed
Dec 5, 2024
Comment on lines
919
to
923
| request_args: Dict[str, Any] = dict( | ||
| method=method, | ||
| url=url, | ||
| headers=headers, | ||
| timeout=self.request_timeout, |
There was a problem hiding this comment.
nit; could use a request.Session in a method .get_session(additional_params) instead of including this all inline
There was a problem hiding this comment.
Much cleaner, thanks for the suggestion
dpeng817
reviewed
Dec 5, 2024
| response.raise_for_status() | ||
| return response.json() | ||
| except RequestException as e: | ||
| self._log.error("Request to Airbyte API failed: %s", e) |
There was a problem hiding this comment.
maybe include the url used + method
dpeng817
reviewed
Dec 5, 2024
|
|
||
| # Taken from Airbyte API documentation | ||
| # https://reference.airbyte.com/reference/createaccesstoken | ||
| SAMPLE_ACCESS_TOKEN = {"access_token": TEST_ACCESS_TOKEN} |
dpeng817
reviewed
Dec 5, 2024
| ) | ||
|
|
||
|
|
||
| def test_refresh_access_token(base_api_mocks: responses.RequestsMock) -> None: |
dpeng817
approved these changes
Dec 5, 2024
maximearmstrong
force-pushed
the
maxime/rework-airbyte-cloud-2
branch
from
8397e6a to
1df12dd
Compare
maximearmstrong
force-pushed
the
maxime/rework-airbyte-cloud-3
branch
from
779d404 to
1e19ef1
Compare
maximearmstrong
force-pushed
the
maxime/rework-airbyte-cloud-3
branch
from
1e19ef1 to
d89aee3
Compare
maximearmstrong
force-pushed
the
maxime/rework-airbyte-cloud-3
branch
from
d89aee3 to
4c7c1b9
Compare
pskinnerthyme
pushed a commit
to pskinnerthyme/dagster
that referenced
this pull request
Dec 16, 2024
…lient (dagster-io#26241) ## Summary & Motivation This PR implements the base `_make_request` method for AirbyteCloudClient and the methods that are required to handle the creation of the access token. About the access token, the logic was taken and reworked from this [previous implementation](dagster-io#23451). TL;DR, Airbyte now requires a client ID and secret to generate an access token - this access token expires every 3 minutes. See more [here](https://reference.airbyte.com/reference/portalairbytecom-deprecation). ## How I Tested These Changes Additional tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary & Motivation
This PR implements the base
_make_requestmethod for AirbyteCloudClient and the methods that are required to handle the creation of the access token.About the access token, the logic was taken and reworked from this previous implementation. TL;DR, Airbyte now requires a client ID and secret to generate an access token - this access token expires every 3 minutes. See more here.
How I Tested These Changes
Additional tests