🎉 New Source: Amplitude #3664

yevhenii-ldv · 2021-05-27T14:16:23Z

What

closes #1457

How

Describe the solution

Pre-merge Checklist

Run integration tests
Publish Docker images

Recommended reading order

test.java
component.ts
the rest

yevhenii-ldv · 2021-05-27T14:40:05Z

/test connector=source-amplitude

🕑 source-amplitude https://github.com/airbytehq/airbyte/actions/runs/882536391
❌ source-amplitude https://github.com/airbytehq/airbyte/actions/runs/882536391

vitaliizazmic · 2021-05-27T15:35:28Z

airbyte-integrations/connectors/source-amplitude/setup.py

+
+MAIN_REQUIREMENTS = [
+    "airbyte-cdk~=0.1",
+]


Were all requirements included here? E.g, there are pendulum and request importing in api.py

These libraries are installed together with airbyte-cdk

vitaliizazmic · 2021-05-27T15:37:56Z

airbyte-integrations/connectors/source-amplitude/acceptance-test-config.yml

+      # Unable to use 'state_path' because Amplitude returns an error when specifying a date in the future.
+      # state_path: "integration_tests/abnormal_state.json"
+      cursor_paths:
+        events: [ "event_time" ]


There are two more incremental streams. Should cursor path be set for them?

In general, it is not at all necessary to specify "cursor_paths" here if it is specified in the directory, however, there is a bug and we must specify the cursor for at least one stream for the test to work.

vitaliizazmic · 2021-05-27T15:39:33Z

airbyte-integrations/connectors/source-amplitude/source_amplitude/api.py

+        yield from respose_data.get(self.name, [])
+
+    def path(self, **kwargs) -> str:
+        return f"/{self.api_version}/{self.name}"


I think, version should be a part of base url.

This is inconvenient because different versions are used.

Suggested change

return f"/{self.api_version}/{self.name}"

@property

def url_base(self) -> str:

return f"https://amplitude.com/api/{self.api_version}/"

WDYT about this?

vitaliizazmic · 2021-05-27T15:40:44Z

airbyte-integrations/connectors/source-amplitude/source_amplitude/api.py

+
+class Cohorts(AmplitudeStream):
+    primary_key = "id"
+    api_version = 3


Why several versions are used?

I used documentation for Amplitude, and there, different versions are used for different streams:

All Cohorts - version 3

Active User Counts - version 2

Export Events - version 2

…/new-amplitude-connector

yevhenii-ldv

I removed the stream Events from the tests, because there is too much data on it. I tried to run it locally and the test only passes if the start_date is 1 or 2 days ago (no more).
Otherwise, the test crashes due to timeout or insufficient memory (since it stores data for comparison).

yevhenii-ldv · 2021-05-28T11:36:07Z

/test connector=source-amplitude

🕑 source-amplitude https://github.com/airbytehq/airbyte/actions/runs/885439055
✅ source-amplitude https://github.com/airbytehq/airbyte/actions/runs/885439055

vitaliizazmic · 2021-05-28T16:24:47Z

LGTM only small change

airbyte-integrations/connectors/source-amplitude/source_amplitude/api.py

airbyte-integrations/connectors/source-amplitude/source_amplitude/source.py

docs/integrations/connector-health.md

yevhenii-ldv · 2021-06-01T09:37:34Z

/test connector=source-amplitude

🕑 source-amplitude https://github.com/airbytehq/airbyte/actions/runs/895528115
✅ source-amplitude https://github.com/airbytehq/airbyte/actions/runs/895528115

…/new-amplitude-connector � Conflicts: � docs/integrations/connector-health.md � tools/bin/ci_credentials.sh

yevhenii-ldv · 2021-06-01T09:50:09Z

/test connector=source-amplitude

🕑 source-amplitude https://github.com/airbytehq/airbyte/actions/runs/895562197
✅ source-amplitude https://github.com/airbytehq/airbyte/actions/runs/895562197

htrueman · 2021-06-01T09:52:51Z

LGTM

sherifnada

Looks great! few questions

sherifnada · 2021-06-01T22:15:11Z

airbyte-integrations/connectors/source-amplitude/source_amplitude/api.py

+        return params
+
+
+class Events(IncrementalAmplitudeStream):


are events returned in ascending order of the cursor field? if so, why don't we checkpoint using checkpoint_interval? This would allow the connector to resume where it left off even if it failed halfway through the sync.

same question for other streams BTW

Also, this would happen automatically if we override the stream_slices method. Because you are doing time slicing here, it's a good fit for slicing and keeps track of state automatically.

I used state_checkpoint_interval, it was equal to 100 for all Incremental streams, but now I slightly corrected, for all I set 10, and for Events I set 1000 (since there are quite a few entries)

sherifnada · 2021-06-01T22:22:10Z

airbyte-integrations/connectors/source-amplitude/source_amplitude/api.py

+    def parse_response(self, response: requests.Response, **kwargs) -> Iterable[Mapping]:
+        response_data = response.json().get("data", [])
+        if response_data:
+            series = response_data["series"][0]


why are we getting the first index in the series list? Can you add a comment explaining it? It's not obvious from the API docs there will be only one record I think https://developers.amplitude.com/docs/dashboard-rest-api#average-session-length

Added comment

…/new-amplitude-connector � Conflicts: � airbyte-integrations/builds.md � docs/SUMMARY.md

yevhenii-ldv · 2021-06-04T13:46:29Z

/test connector=source-amplitude

🕑 source-amplitude https://github.com/airbytehq/airbyte/actions/runs/906457173
✅ source-amplitude https://github.com/airbytehq/airbyte/actions/runs/906457173

…/new-amplitude-connector

yevhenii-ldv · 2021-06-08T07:22:33Z

/publish connector=connectors/source-amplitude

🕑 connectors/source-amplitude https://github.com/airbytehq/airbyte/actions/runs/917468801
✅ connectors/source-amplitude https://github.com/airbytehq/airbyte/actions/runs/917468801

yevhenii-ldv · 2021-06-08T07:29:37Z

/test connector=source-amplitude

🕑 source-amplitude https://github.com/airbytehq/airbyte/actions/runs/917487410
✅ source-amplitude https://github.com/airbytehq/airbyte/actions/runs/917487410

New Amplitude Source: Full Refresh and Incremental Sync

44d91ab

auto-assign bot requested review from ChristopheDuong and michel-tricot May 27, 2021 14:16

yevhenii-ldv requested review from htrueman, keu, po3na4skld, vitaliizazmic, vovavovavovavova and Zirochkaa and removed request for michel-tricot and ChristopheDuong May 27, 2021 14:16

format

ffafdcf

vitaliizazmic reviewed May 27, 2021

View reviewed changes

ykurochkin added 4 commits May 28, 2021 13:50

fix events stream api

f239028

remove Events stream from Acceptance tests (too much records)

a65cc5d

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

cec4d08

…/new-amplitude-connector

fix cursor_paths for incremental test

675abf4

yevhenii-ldv commented May 28, 2021

View reviewed changes

yevhenii-ldv requested a review from vitaliizazmic May 28, 2021 15:29

vitaliizazmic approved these changes May 28, 2021

View reviewed changes

yevhenii-ldv requested review from sherifnada and davinchia May 30, 2021 23:15

htrueman reviewed May 31, 2021

View reviewed changes

update code after review

ac899c6

ykurochkin added 2 commits June 1, 2021 12:45

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

ca12028

…/new-amplitude-connector � Conflicts: � docs/integrations/connector-health.md � tools/bin/ci_credentials.sh

delete back connector-health.md

56a92c7

yevhenii-ldv requested a review from htrueman June 1, 2021 09:50

htrueman approved these changes Jun 1, 2021

View reviewed changes

add connector to builds.md

884b60b

sherifnada suggested changes Jun 1, 2021

View reviewed changes

sherifnada mentioned this pull request Jun 3, 2021

Amplitude and Hubspot connector #3854

Closed

ykurochkin added 2 commits June 4, 2021 16:41

update api.py, add CHANGELOG.md file

6928075

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

e9fff1d

…/new-amplitude-connector � Conflicts: � airbyte-integrations/builds.md � docs/SUMMARY.md

yevhenii-ldv requested a review from sherifnada June 4, 2021 13:46

sherifnada approved these changes Jun 4, 2021

View reviewed changes

ykurochkin added 2 commits June 8, 2021 10:17

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

b1509e6

…/new-amplitude-connector

bump version

e76c8e0

github-actions bot added the area/documentation Improvements or additions to documentation label Jun 8, 2021

yevhenii-ldv merged commit a90e5f0 into master Jun 8, 2021

yevhenii-ldv deleted the ykurochkin/new-amplitude-connector branch June 8, 2021 07:36

igrankova added connectors/source/amplitude connectors/sources-api labels Jan 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🎉 New Source: Amplitude #3664

🎉 New Source: Amplitude #3664

yevhenii-ldv commented May 27, 2021

yevhenii-ldv commented May 27, 2021 •

edited by github-actions bot

Loading

vitaliizazmic May 27, 2021

yevhenii-ldv May 28, 2021

vitaliizazmic May 27, 2021

yevhenii-ldv May 28, 2021

vitaliizazmic May 27, 2021

yevhenii-ldv May 28, 2021

vitaliizazmic May 28, 2021

vitaliizazmic May 28, 2021

vitaliizazmic May 27, 2021

yevhenii-ldv May 28, 2021

yevhenii-ldv left a comment

yevhenii-ldv commented May 28, 2021 •

edited by github-actions bot

Loading

vitaliizazmic commented May 28, 2021

yevhenii-ldv commented Jun 1, 2021 •

edited by github-actions bot

Loading

yevhenii-ldv commented Jun 1, 2021 •

edited by github-actions bot

Loading

htrueman commented Jun 1, 2021

sherifnada left a comment

sherifnada Jun 1, 2021

sherifnada Jun 1, 2021

sherifnada Jun 1, 2021

yevhenii-ldv Jun 4, 2021

sherifnada Jun 1, 2021

yevhenii-ldv Jun 4, 2021

yevhenii-ldv commented Jun 4, 2021 •

edited by github-actions bot

Loading

yevhenii-ldv commented Jun 8, 2021 •

edited by github-actions bot

Loading

yevhenii-ldv commented Jun 8, 2021 •

edited by github-actions bot

Loading

-        return f"/{self.api_version}/{self.name}"
+        @property
+        def url_base(self) -> str:
+            return f"https://amplitude.com/api/{self.api_version}/"

🎉 New Source: Amplitude #3664

🎉 New Source: Amplitude #3664

Conversation

yevhenii-ldv commented May 27, 2021

What

How

Pre-merge Checklist

Recommended reading order

yevhenii-ldv commented May 27, 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

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 left a comment

Choose a reason for hiding this comment

yevhenii-ldv commented May 28, 2021 • edited by github-actions bot Loading

vitaliizazmic commented May 28, 2021

yevhenii-ldv commented Jun 1, 2021 • edited by github-actions bot Loading

yevhenii-ldv commented Jun 1, 2021 • edited by github-actions bot Loading

htrueman commented Jun 1, 2021

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

yevhenii-ldv commented Jun 4, 2021 • edited by github-actions bot Loading

yevhenii-ldv commented Jun 8, 2021 • edited by github-actions bot Loading

yevhenii-ldv commented Jun 8, 2021 • edited by github-actions bot Loading

yevhenii-ldv commented May 27, 2021 •

edited by github-actions bot

Loading

yevhenii-ldv commented May 28, 2021 •

edited by github-actions bot

Loading

yevhenii-ldv commented Jun 1, 2021 •

edited by github-actions bot

Loading

yevhenii-ldv commented Jun 1, 2021 •

edited by github-actions bot

Loading

yevhenii-ldv commented Jun 4, 2021 •

edited by github-actions bot

Loading

yevhenii-ldv commented Jun 8, 2021 •

edited by github-actions bot

Loading

yevhenii-ldv commented Jun 8, 2021 •

edited by github-actions bot

Loading