[12/n][dagster-airbyte] Implement sync_and_poll method in AirbyteCloudClient #26431

maximearmstrong · 2024-12-12T05:41:41Z

Summary & Motivation

Implement full sync and poll process in AirbyteCloudClient. This will be used in a subsequent PR in AirbyteCloudWorkspace.sync_and_poll to materialize Airbyte assets.

How I Tested These Changes

Additional tests with BK

maximearmstrong · 2024-12-12T05:41:50Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

maximearmstrong · 2024-12-18T00:53:03Z

python_modules/libraries/dagster-airbyte/dagster_airbyte/translator.py

+
+
+@deprecated(breaking_version="1.10", additional_warn_text="Use `AirbyteJobStatusType` instead.")
+class AirbyteState:


This enum is renamed to be AirbyteJobStatusType, which reflects Airbyte's ontology and our naming convention in other integrations.

Kept for backcompat and to be removed in 1.10 because it was exposed in __init__.py

Is 1.10 the right thing to say here; since dagster-airbyte is on a pre 1.0 release ?

maximearmstrong · 2024-12-18T00:56:18Z

python_modules/libraries/dagster-airbyte/dagster_airbyte_tests/experimental/conftest.py

@@ -27,7 +27,9 @@
 TEST_STREAM_NAME = "test_stream"
 TEST_SELECTED = True
 TEST_JSON_SCHEMA = {}
-TEST_JOB_ID = "3fa85f64-5717-4562-b3fc-2c963f66afa6"
+TEST_JOB_ID = 12345


Fixes an error in Airbyte's example in the API documentation here. The json example uses a UUID as the Job ID, but the documentation mentions that the ID is an int.

maximearmstrong · 2024-12-18T00:59:55Z

python_modules/libraries/dagster-airbyte/dagster_airbyte_tests/test_cloud_resources.py

@@ -50,7 +50,12 @@ def test_trigger_connection_fail() -> None:
 @responses.activate
 @pytest.mark.parametrize(
    "state",
-    [AirbyteState.SUCCEEDED, AirbyteState.CANCELLED, AirbyteState.ERROR, "unrecognized"],
+    [


Nothing is changed in the legacy tests except the name of the enum

dpeng817 · 2024-12-18T16:25:53Z

python_modules/libraries/dagster-airbyte/dagster_airbyte/resources.py

+                    )
+
+                time.sleep(poll_interval)
+                poll_job_details = self.get_job_details(job.id)


nit; why make the caller initialize the job from the details. Feels like we should just return it directly

dpeng817 · 2024-12-18T16:29:18Z

python_modules/libraries/dagster-airbyte/dagster_airbyte_tests/experimental/test_resources.py

+    if status == TEST_UNRECOGNIZED_AIRBYTE_JOB_STATUS_TYPE:
+        base_api_mocks.add(
+            method=responses.DELETE,
+            url=test_job_api_url,
+            status=200,
+            json=get_job_details_sample(status=AirbyteJobStatusType.CANCELLED),
+        )


confused by this - why is this necessary?

We have another test dedicated to cancel_on_termination, so let's remove this part of the test when testing statuses - responses.RequestsMock yells if a mocked request is not called in a test, so it must be added only when the job is cancelled.

dpeng817 · 2024-12-18T16:30:58Z

python_modules/libraries/dagster-airbyte/dagster_airbyte_tests/experimental/test_resources.py

+    if status in [AirbyteJobStatusType.ERROR, AirbyteJobStatusType.FAILED]:
+        with pytest.raises(Failure, match="Job failed"):
+            client.sync_and_poll(connection_id=TEST_CONNECTION_ID, poll_interval=0)
+
+    elif status == AirbyteJobStatusType.CANCELLED:
+        with pytest.raises(Failure, match="Job was cancelled"):
+            client.sync_and_poll(connection_id=TEST_CONNECTION_ID, poll_interval=0)
+
+    elif status == TEST_UNRECOGNIZED_AIRBYTE_JOB_STATUS_TYPE:
+        with pytest.raises(Failure, match="unexpected state"):
+            client.sync_and_poll(connection_id=TEST_CONNECTION_ID, poll_interval=0)
+        assert_rest_api_call(
+            call=base_api_mocks.calls[-1], endpoint=test_job_endpoint, method=responses.DELETE
+        )
+
+    else:
+        result = client.sync_and_poll(connection_id=TEST_CONNECTION_ID, poll_interval=0)
+        assert result == AirbyteOutput(
+            job_details=get_job_details_sample(AirbyteJobStatusType.SUCCEEDED),
+            connection_details=SAMPLE_CONNECTION_DETAILS,
+        )


nit; this could all be part of the parameterization. I actually had to do something similar recently, and I found an optional_pytest_raise to be really useful here: https://github.com/dagster-io/dagster/pull/26545/files#diff-bb4b77564796719ee9a5eaf83ecd67f54f4f16b8b7724f9348d1a38dac76800d

Updated in 5df711b

dpeng817 · 2024-12-18T16:32:56Z

python_modules/libraries/dagster-airbyte/dagster_airbyte_tests/experimental/test_resources.py

+    if cancel_on_termination:
+        assert_rest_api_call(
+            call=base_api_mocks.calls[-1], endpoint=test_job_endpoint, method=responses.DELETE
+        )
+    else:
+        # If we don't cancel on termination, the last call will be a call to fetch the job details
+        assert_rest_api_call(
+            call=base_api_mocks.calls[-1], endpoint=test_job_endpoint, method=responses.GET
+        )


really don't love these complex ternaries in test body IMO. Just really hard to read, I think this should just be a part of the parameterization.

That's a good call - updated in 5df711b

dpeng817

Some feedback / comments but nothing review blocking

This was referenced Dec 12, 2024

[9/n][dagster-airbyte] Implement base sync method in AirbyteCloudClient #26428

Merged

[10/n][dagster-airbyte] Implement get_job_details method in AirbyteCloudClient #26429

Merged

This was referenced Dec 12, 2024

[11/n][dagster-airbyte] Implement cancel_job method in AirbyteCloudClient #26430

Merged

[13/n][dagster-airbyte] Implement airbyte_assets and build_airbyte_assets_definitions #26432

Merged

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-11 branch from 01c67e0 to 6bcbaf3 Compare December 13, 2024 00:03

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-12 branch from 4b9b2c1 to 6585306 Compare December 13, 2024 00:04

maximearmstrong changed the title ~~[dagster-airbyte] Implement sync_and_poll method in AirbyteCloudClient~~ [12/n][dagster-airbyte] Implement sync_and_poll method in AirbyteCloudClient Dec 16, 2024

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-11 branch from 6bcbaf3 to 5b2f698 Compare December 17, 2024 23:13

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-12 branch from 6585306 to e8d86fc Compare December 17, 2024 23:13

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-11 branch from 5b2f698 to 48f6c5e Compare December 18, 2024 00:48

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-12 branch from e8d86fc to 15f5b2b Compare December 18, 2024 00:48

maximearmstrong mentioned this pull request Dec 18, 2024

[14/n][dagster-airbyte] Implement materialization method for AirbyteCloudWorkspace #26559

Merged

maximearmstrong commented Dec 18, 2024

View reviewed changes

maximearmstrong self-assigned this Dec 18, 2024

maximearmstrong marked this pull request as ready for review December 18, 2024 01:21

maximearmstrong requested review from dpeng817 and benpankow December 18, 2024 01:21

dpeng817 reviewed Dec 18, 2024

View reviewed changes

dpeng817 approved these changes Dec 18, 2024

View reviewed changes

dpeng817 reviewed Dec 18, 2024

View reviewed changes

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-11 branch from 48f6c5e to 438b2bc Compare December 18, 2024 22:42

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-12 branch from 1b64ddb to 860d858 Compare December 18, 2024 22:42

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-11 branch from 438b2bc to 2255aff Compare December 19, 2024 01:19

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-12 branch from 860d858 to c7fbd62 Compare December 19, 2024 01:19

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-11 branch from 2255aff to e4f7ec5 Compare December 19, 2024 03:29

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-12 branch from c7fbd62 to 5caf85f Compare December 19, 2024 03:29

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-11 branch from e4f7ec5 to 0f39290 Compare December 19, 2024 04:01

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-12 branch from 5caf85f to f888a06 Compare December 19, 2024 04:01

This was referenced Dec 19, 2024

[dagster-airbyte][docs] Update airbyte API docs with asset decorator, factory and materialization method #26583

Merged

[docs] Move legacy Airbyte Cloud docs #26584

Merged

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-11 branch from 0f39290 to 159a035 Compare December 19, 2024 15:50

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-12 branch from f888a06 to 90848ca Compare December 19, 2024 15:50

maximearmstrong mentioned this pull request Dec 19, 2024

[docs] Add docs for reworked Airbyte Cloud API #26595

Merged

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-11 branch from 159a035 to 1ae39d0 Compare December 26, 2024 15:30

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-12 branch from 90848ca to 94bb6cc Compare December 26, 2024 15:30

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-11 branch from 1ae39d0 to 797ff67 Compare December 26, 2024 15:44

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-12 branch from 94bb6cc to 7009a22 Compare December 26, 2024 15:45

Base automatically changed from maxime/rework-airbyte-cloud-11 to master December 26, 2024 16:07

maximearmstrong added 6 commits December 26, 2024 11:08

[dagster-airbyte] Implement sync_and_poll method in AirbyteCloudClient

16a4810

Update AirbyteCloudClient.sync_and_poll

2e9e045

Add tests

3c76dca

Update tests

875d62f

Update tests

3c04d0b

Add comment in sync_and_poll

fa56484

maximearmstrong force-pushed the maxime/rework-airbyte-cloud-12 branch from 7009a22 to fa56484 Compare December 26, 2024 16:08

maximearmstrong merged commit 2384e5b into master Dec 26, 2024
1 check passed

maximearmstrong deleted the maxime/rework-airbyte-cloud-12 branch December 26, 2024 16:31

maximearmstrong mentioned this pull request Dec 26, 2024

[dagster-airbyte][docs] Migrate Airbyte cloud docs to new doc site #26724

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[12/n][dagster-airbyte] Implement sync_and_poll method in AirbyteCloudClient #26431

[12/n][dagster-airbyte] Implement sync_and_poll method in AirbyteCloudClient #26431

maximearmstrong commented Dec 12, 2024 •

edited

Loading

maximearmstrong commented Dec 12, 2024 •

edited

Loading

maximearmstrong Dec 18, 2024 •

edited

Loading

dpeng817 Dec 18, 2024 •

edited

Loading

maximearmstrong Dec 18, 2024

maximearmstrong Dec 18, 2024

dpeng817 Dec 18, 2024

dpeng817 Dec 18, 2024 •

edited

Loading

maximearmstrong Dec 18, 2024 •

edited

Loading

dpeng817 Dec 18, 2024

maximearmstrong Dec 19, 2024

dpeng817 Dec 18, 2024

maximearmstrong Dec 19, 2024

dpeng817 left a comment



		@deprecated(breaking_version="1.10", additional_warn_text="Use `AirbyteJobStatusType` instead.")
		class AirbyteState:

[12/n][dagster-airbyte] Implement sync_and_poll method in AirbyteCloudClient #26431

[12/n][dagster-airbyte] Implement sync_and_poll method in AirbyteCloudClient #26431

Conversation

maximearmstrong commented Dec 12, 2024 • edited Loading

Summary & Motivation

How I Tested These Changes

maximearmstrong commented Dec 12, 2024 • edited Loading

maximearmstrong Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

dpeng817 Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

maximearmstrong Dec 18, 2024

Choose a reason for hiding this comment

maximearmstrong Dec 18, 2024

Choose a reason for hiding this comment

dpeng817 Dec 18, 2024

Choose a reason for hiding this comment

dpeng817 Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

maximearmstrong Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

dpeng817 Dec 18, 2024

Choose a reason for hiding this comment

maximearmstrong Dec 19, 2024

Choose a reason for hiding this comment

dpeng817 Dec 18, 2024

Choose a reason for hiding this comment

maximearmstrong Dec 19, 2024

Choose a reason for hiding this comment

dpeng817 left a comment

Choose a reason for hiding this comment

maximearmstrong commented Dec 12, 2024 •

edited

Loading

maximearmstrong commented Dec 12, 2024 •

edited

Loading

maximearmstrong Dec 18, 2024 •

edited

Loading

dpeng817 Dec 18, 2024 •

edited

Loading

dpeng817 Dec 18, 2024 •

edited

Loading

maximearmstrong Dec 18, 2024 •

edited

Loading