🎉 New Source: Iterable #2895

yevhenii-ldv · 2021-04-15T09:54:27Z

What

Creating new source Iterable based on Base Python

closes #2412

Pre-merge Checklist

Run integration tests
Publish Docker images

Recommended reading order

test.java
component.ts
the rest

…/new-connector-iterable

…ehq/airbyte into ykurochkin/new-connector-iterable � Conflicts: � airbyte-integrations/connectors/source-iterable/setup.py

This reverts commit e30d089

yevhenii-ldv · 2021-04-15T10:04:38Z

/test connector=source-iterable

🕑 source-iterable https://github.com/airbytehq/airbyte/actions/runs/751604345
✅ source-iterable https://github.com/airbytehq/airbyte/actions/runs/751604345

…/new-connector-iterable

vitaliizazmic · 2021-04-16T14:54:33Z

airbyte-integrations/connectors/source-iterable/source_iterable/source.py

+            list(Lists(authenticator=authenticator)._list_records(stream_state={}))
+            return True, None
+        except Exception as e:
+            return False, e


Maybe it's better to add more detail message.

arhip11 · 2021-04-16T15:48:38Z

airbyte-integrations/connectors/source-iterable/source_iterable/api.py

+        :return:
+        """
+
+        for templete in self.template_types:


Is it a typo: templete vs template?

arhip11 · 2021-04-16T15:48:49Z

airbyte-integrations/connectors/source-iterable/source_iterable/api.py

+                args = {"stream_state": stream_state}
+
+                request = self._create_prepared_request(
+                    path=self.path().format(templete, message),


sherifnada

This looks great! very easy to follow. Few comments and we should be ready to go. Think we can merge on Monday?

sherifnada · 2021-04-18T02:52:10Z

airbyte-integrations/connectors/source-iterable/source_iterable/api.py

+    url_base = "https://api.iterable.com/api/"
+
+    # Hardcode the value because it is not returned from the API
+    BACKOFF_TIME_CONSTANT = 10.0


is the expected behavior to wait 10 seconds? BTW the default behavior for backing off in the framework is an exponential backoff. So if that behavior is acceptable, we don't need this and we don't need to implement backoff_time.

I saw the default behavior for backing off and find that we can't use the default backoff for 429 error, because we just ignore it.

airbyte/airbyte-integrations/bases/base-python/base_python/sdk/streams/rate_limiting.py

Lines 45 to 47 in 3e68438

def should_give_up(exc):

# If a 4XX error makes it this far, it means it was unexpected and probably consistent, so we shouldn't back off

return exc.response is not None and 400 <= exc.response.status_code < 500

Ah I see. I have a fix for this in the latest refactor of the framework -- this is a bug. It should have and exc.response.status_code != 429. We'll be able to refactor this soon to rely on the default backoff behavior.

sherifnada · 2021-04-18T02:58:00Z

airbyte-integrations/connectors/source-iterable/source_iterable/api.py

+        parent_stream_record: Mapping = None,
+    ) -> MutableMapping[str, Any]:
+
+        params = super(IterableExportStream, self).request_params(stream_state=stream_state)


are the params to super needed?

Suggested change

params = super(IterableExportStream, self).request_params(stream_state=stream_state)

params = super().request_params(stream_state=stream_state)

sherifnada · 2021-04-18T02:59:20Z

airbyte-integrations/connectors/source-iterable/source_iterable/api.py

+        return params
+
+    def path(self, **kwargs) -> str:
+        return f"/export/data.json?dataTypeName={self.data_field}"


should dataTypeName be specified in request_params instead?

sherifnada · 2021-04-18T03:03:35Z

airbyte-integrations/connectors/source-iterable/source_iterable/auth.py

+    def get_auth_header(self) -> Mapping[str, Any]:
+        return {}
+
+    def get_auth_params(self) -> Mapping[str, Any]:


if auth is not in the headers, is it worth creating an entire Authenticator class? Why don't we just pass the token in the constructor parameters for each stream and then the base IterableStream class can add it in the request_params method? authenticator is an optional parameter for HTTP streams anyways.

yeah, good idea, updated

sherifnada · 2021-04-18T03:13:12Z

airbyte-integrations/connectors/source-iterable/source_iterable/api.py

+        :return:
+        """
+
+        for template in self.template_types:


hmmm I see why you had to do this...not ideal that we are overriding an _ method. Working on an upcoming refactor that will hopefully make this much cleaner.

I think I solved this problem and used only the read_stream method with setting stream_params, please look at the PR after the update.

* Iterable #2897: support incremental sync Co-authored-by: ykurochkin <y.kurochkin@zazmic.com>

yevhenii-ldv · 2021-04-19T12:49:29Z

/test connector=source-iterable

🕑 source-iterable https://github.com/airbytehq/airbyte/actions/runs/763594203
✅ source-iterable https://github.com/airbytehq/airbyte/actions/runs/763594203

…/new-connector-iterable

yevhenii-ldv · 2021-04-20T12:33:31Z

/publish connector=connectors/source-iterable

🕑 connectors/source-iterable https://github.com/airbytehq/airbyte/actions/runs/767126491
✅ connectors/source-iterable https://github.com/airbytehq/airbyte/actions/runs/767126491

yevhenii-ldv · 2021-04-20T12:46:58Z

/test connector=source-iterable

🕑 source-iterable https://github.com/airbytehq/airbyte/actions/runs/767163745
✅ source-iterable https://github.com/airbytehq/airbyte/actions/runs/767163745

ykurochkin and others added 11 commits April 13, 2021 17:44

base version of Iterable connector

d007952

Merge branch 'master' of github.com:airbytehq/airbyte into ykurochkin…

f4ec1a3

…/new-connector-iterable

Merge branch 'master' of github.com:airbytehq/airbyte into ykurochkin…

cb01bef

…/new-connector-iterable

update base-python for iterable Dockerfile

00ec8e4

fix import logic

e30d089

fix versions

3454c71

remove entrypoint

580c6df

Merge branch 'master' of github.com:airbytehq/airbyte into ykurochkin…

bcdcb87

…/new-connector-iterable

add docs to Iterable source, update code

30bf0c2

Merge branch 'ykurochkin/new-connector-iterable' of github.com:airbyt…

15c6dec

…ehq/airbyte into ykurochkin/new-connector-iterable � Conflicts: � airbyte-integrations/connectors/source-iterable/setup.py

Revert "fix import logic"

d8cd49e

This reverts commit e30d089

yevhenii-ldv requested review from keu, arhip11, Zirochkaa and vitaliizazmic April 15, 2021 09:54

auto-assign bot requested review from ChristopheDuong and jrhizor April 15, 2021 09:54

yevhenii-ldv removed request for ChristopheDuong and jrhizor April 15, 2021 09:55

Merge branch 'master' of github.com:airbytehq/airbyte into ykurochkin…

cee6cbd

…/new-connector-iterable

vitaliizazmic reviewed Apr 16, 2021

View reviewed changes

add more information for user on check method

ae38d89

arhip11 reviewed Apr 16, 2021

View reviewed changes

removed the typo

5c8ff29

yevhenii-ldv linked an issue Apr 16, 2021 that may be closed by this pull request

Source Iterable: Implement Read #2739

Closed

yevhenii-ldv requested a review from sherifnada April 16, 2021 15:57

sherifnada approved these changes Apr 18, 2021

View reviewed changes

yevhenii-ldv mentioned this pull request Apr 19, 2021

Source Iterable: support incremental sync #2916

Merged

2 tasks

Source Iterable: support incremental sync (#2916)

5f33ff1

* Iterable #2897: support incremental sync Co-authored-by: ykurochkin <y.kurochkin@zazmic.com>

update after Sherif review PR

3c255d1

sherifnada approved these changes Apr 19, 2021

View reviewed changes

ykurochkin added 2 commits April 20, 2021 15:24

Merge branch 'master' of github.com:airbytehq/airbyte into ykurochkin…

4ca5c5c

…/new-connector-iterable

bumping version

e96f004

yevhenii-ldv merged commit f8286e4 into master Apr 20, 2021

yevhenii-ldv deleted the ykurochkin/new-connector-iterable branch April 20, 2021 13:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🎉 New Source: Iterable #2895

🎉 New Source: Iterable #2895

yevhenii-ldv commented Apr 15, 2021

yevhenii-ldv commented Apr 15, 2021 •

edited by github-actions bot

Loading

vitaliizazmic Apr 16, 2021

yevhenii-ldv Apr 16, 2021

arhip11 Apr 16, 2021

arhip11 Apr 16, 2021

yevhenii-ldv Apr 16, 2021

sherifnada left a comment

sherifnada Apr 18, 2021

yevhenii-ldv Apr 19, 2021

sherifnada Apr 19, 2021

sherifnada Apr 18, 2021

sherifnada Apr 18, 2021

sherifnada Apr 18, 2021

yevhenii-ldv Apr 19, 2021

sherifnada Apr 18, 2021

yevhenii-ldv Apr 19, 2021

yevhenii-ldv commented Apr 19, 2021 •

edited by github-actions bot

Loading

yevhenii-ldv commented Apr 20, 2021 •

edited by github-actions bot

Loading

yevhenii-ldv commented Apr 20, 2021 •

edited by github-actions bot

Loading

	def should_give_up(exc):
	# If a 4XX error makes it this far, it means it was unexpected and probably consistent, so we shouldn't back off
	return exc.response is not None and 400 <= exc.response.status_code < 500

	params = super(IterableExportStream, self).request_params(stream_state=stream_state)
	params = super().request_params(stream_state=stream_state)

🎉 New Source: Iterable #2895

🎉 New Source: Iterable #2895

Conversation

yevhenii-ldv commented Apr 15, 2021

What

Pre-merge Checklist

Recommended reading order

yevhenii-ldv commented Apr 15, 2021 • edited by github-actions bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sherifnada left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yevhenii-ldv commented Apr 19, 2021 • edited by github-actions bot Loading

yevhenii-ldv commented Apr 20, 2021 • edited by github-actions bot Loading

yevhenii-ldv commented Apr 20, 2021 • edited by github-actions bot Loading

yevhenii-ldv commented Apr 15, 2021 •

edited by github-actions bot

Loading

yevhenii-ldv commented Apr 19, 2021 •

edited by github-actions bot

Loading

yevhenii-ldv commented Apr 20, 2021 •

edited by github-actions bot

Loading

yevhenii-ldv commented Apr 20, 2021 •

edited by github-actions bot

Loading