🎉 New source: Native Hubspot connector #2215

eugene-kulak · 2021-02-25T20:20:11Z

What

closes #2195
closes #2215

How

Describe the solution

Pre-merge Checklist

Run integration tests
Publish Docker images

Recommended reading order

test.java
component.ts
the rest

TODOs:

Use interval
check if pagination is possible on contacts by company stream

eugene-kulak · 2021-02-26T07:36:25Z

/test connector=source-hubspot

🕑 source-hubspot https://github.com/airbytehq/airbyte/actions/runs/602034821
❌ source-hubspot https://github.com/airbytehq/airbyte/actions/runs/602034821

eugene-kulak · 2021-02-26T07:52:19Z

/test connector=source-hubspot

🕑 source-hubspot https://github.com/airbytehq/airbyte/actions/runs/602065374
❌ source-hubspot https://github.com/airbytehq/airbyte/actions/runs/602065374

sherifnada

Looks good so far, I really like the breakdown of classes between API, StreamAPI, and CRMObjects 🚀

sherifnada · 2021-02-27T00:46:43Z

airbyte-config/init/src/main/resources/seed/source_definitions.yaml

@@ -78,8 +78,13 @@
  dockerRepository: airbyte/source-facebook-marketing
  dockerImageTag: 0.1.3
  documentationUrl: https://hub.docker.com/r/airbyte/source-facebook-marketing
- sourceDefinitionId: 57eb1576-8f52-463d-beb6-2e107cdf571d
+- sourceDefinitionId: 36c891d9-4bd9-43ac-bad2-10e12756272c


We should only make the new version available once incremental is complete. We can then also completely remove the old hubspot connector (rather than rename it).

my intention was to add a completely new connector, then release it and drop the one that based on singer. But maybe accidentally renamed the old instead, not sure, need to check

Important thing is not to have 2 hubspot connectors in Airbyte at the same time. the user only cares about syncing data from hubspot, Singer vs. native is an implementation detail

clear, I have removed the old connector

sherifnada · 2021-02-27T01:10:48Z

airbyte-integrations/connectors/source-hubspot/source_hubspot/api.py

+            return {}
+
+        props = {}
+        data = self._api.get(f"/properties/v2/{self.entity}/properties")


can you add context on what this is doing as a comment?

I've looked at the API docs for a bit and it's not obvious to me what this is doing. it seems like it's adding the type of the property based on dynamically getting it from the Hubspot API? is this related to this ticket? why wouldn't we know the type of the property ahead of time?

This is bringing me to a different point: can you clarify as a docstring at the top of this class what the different APIs we are using are, and point to the API docs? This will enable me as the reader to know exactly which API docs to look at (Hubspot has multiple)

most objects in Hubspot have attributes located in the properties field, there are two types:

default (built-in, depends on the version of API)

custom (user-defined)

because the exact list and types are dynamic we retrieve them at runtime. Technically we don't need to know type of attribute here, but for building a schema object we will need it (#2219 )

good idea about adding docs, will polish the PR soon

sherifnada · 2021-02-27T01:19:17Z

airbyte-integrations/connectors/source-hubspot/source_hubspot/api.py

+    entity = None
+
+    more_key = None
+    data_path = "results"


data_path is a bit misleading as a name since it's a single key, not a path in the JSON. Can we use something like data_field?

sherifnada · 2021-02-27T01:20:21Z

airbyte-integrations/connectors/source-hubspot/source_hubspot/api.py

+    page_filter = "offset"
+    page_field = "offset"
+
+    chunk_size = 1000 * 60 * 60 * 24  # TODO: use interval


I'm adding all TODOs to a checklist in the PR description so we don't miss these before merging

sherifnada · 2021-02-27T01:22:54Z

airbyte-integrations/connectors/source-hubspot/source_hubspot/api.py

+        params = {
+            "archived": str(self._include_archived_only).lower(),
+        }
+        for record in self.read(partial(self._api.get, url=self.url), params):


yield from?

sherifnada · 2021-02-27T01:26:41Z

airbyte-integrations/connectors/source-hubspot/source_hubspot/api.py

+            if response.get(self.data_path) is None:
+                raise RuntimeError("Unexpected API response: {} not in {}".format(self.data_path, response.keys()))
+
+            for row in response[self.data_path]:


yield from?

sherifnada · 2021-02-27T01:29:15Z

airbyte-integrations/connectors/source-hubspot/source_hubspot/api.py

+    url = "/crm/v3/objects/quotes"
+
+
+class TicketsAPI(CRMObjectsAPI):


why does every class have API in the name? Shouldn't it just be the API class and then everything else is a stream?

sherifnada · 2021-02-27T01:29:25Z

airbyte-integrations/connectors/source-hubspot/source_hubspot/api.py

+
+    def _contacts_by_company(self, company_id):
+        url = "/companies/v2/companies/{pk}/vids".format(pk=company_id)
+        # FIXME: check if pagination is possible


added to todo list

sherifnada · 2021-02-27T01:33:18Z

docs/integrations/sources/hubspot.md

+* Campaigns
+* Companies
+* Contact Lists
+* Contacts


can you link to these in the docs? You don't have to link to each one individually, you can just link to one page at the top containing all of themnm

sherifnada · 2021-02-27T01:39:29Z

airbyte-integrations/connectors/source-hubspot/source_hubspot/api.py

+        if data.get(path) is None:
+            raise RuntimeError("Unexpected API response: {} not in {}".format(path, data.keys()))
+
+        for row in data[path]:


is the point of this stream to be a join table between contacts and companies? can't we put this information in the contacts stream as a field?

keu · 2021-03-12T20:16:42Z

/test connector=source-hubspot

🕑 source-hubspot https://github.com/airbytehq/airbyte/actions/runs/647324504
❌ source-hubspot https://github.com/airbytehq/airbyte/actions/runs/647324504

sherifnada · 2021-03-12T22:10:35Z

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

+]
+
+TEST_REQUIREMENTS = [
+    "airbyte-python-test",


airbyte-python-test is not needed

sherifnada · 2021-03-12T22:26:32Z

docs/integrations/sources/hubspot.md

@@ -34,5 +50,5 @@ The connector is restricted by normal Hubspot [rate limitations](https://legacyd

 * For api key auth, in Hubspot, for the account to go settings -&gt; integrations \(under the account banner\) -&gt; api key. If you already have an api key you can use that. Otherwise generated a new one.


As far as I can tell, this connector/airbyte would need to implement oauth for a client to be able to obtain oauth credentials. Since we don't do this yet, can you remove the oauth section from the docs and create an issue to support it once we have oauth? blocked on this issue: #768

* add incremental * add incremental * polishing * update docs * fix docstring * clean up * fix incremental bookmark access * fix incremental tests * clean up * add custom test for incremental, improve logging * format * Update airbyte-integrations/connectors/source-hubspot/source_hubspot/api.py Co-authored-by: Eugene Kulak <kulak.eugene@gmail.com> Co-authored-by: Sherif A. Nada <snadalive@gmail.com>

* fix error reporting and add unit tests * fix test and refactor cursor fields * format Co-authored-by: Eugene Kulak <kulak.eugene@gmail.com>

keu · 2021-03-19T21:39:25Z

/test connector=source-hubspot

🕑 source-hubspot https://github.com/airbytehq/airbyte/actions/runs/669319473
❌ source-hubspot https://github.com/airbytehq/airbyte/actions/runs/669319473

…native

keu · 2021-03-19T22:39:18Z

/test connector=source-hubspot

🕑 source-hubspot https://github.com/airbytehq/airbyte/actions/runs/669438507
❌ source-hubspot https://github.com/airbytehq/airbyte/actions/runs/669438507

keu · 2021-03-20T05:07:03Z

/test connector=source-hubspot

🕑 source-hubspot https://github.com/airbytehq/airbyte/actions/runs/670098223
✅ source-hubspot https://github.com/airbytehq/airbyte/actions/runs/670098223

keu · 2021-03-20T05:25:30Z

/publish connector=source-hubspot

❌ source-hubspot https://github.com/airbytehq/airbyte/actions/runs/670128390

keu · 2021-03-20T05:25:59Z

/publish connector=connectors/source-hubspot

🕑 connectors/source-hubspot https://github.com/airbytehq/airbyte/actions/runs/670129400
✅ connectors/source-hubspot https://github.com/airbytehq/airbyte/actions/runs/670129400

ykurochkin and others added 4 commits February 23, 2021 11:54

#2150 Issue: created native connector with schema folder populated

49cb4cb

#2150 Issue: make format code

5423d13

first version

10a106b

fix few issues

e667fd0

eugene-kulak changed the base branch from master to ykurochkin/hubspot-create-native-connector-with-schema-folder-populated February 25, 2021 20:20

eugene-kulak linked an issue Feb 25, 2021 that may be closed by this pull request

Source Hubspot: native client should support all streams in full sync mode #2195

Closed

3 tasks

eugene-kulak self-assigned this Feb 25, 2021

eugene-kulak requested a review from yevhenii-ldv February 25, 2021 20:32

eugene-kulak added 8 commits February 26, 2021 01:18

fix issues

10af6c9

fix read issue

2e47519

format

a180e71

docs

21152c6

docker tags

3dc6556

extend configured catalog for testing

66b03d0

fix source definitions

2bc9f01

format

0611ac7

eugene-kulak added 2 commits February 26, 2021 09:50

fix call rate issue, add backoff for retry after

42f24bb

add general backoff

4479e02

yevhenii-ldv approved these changes Feb 26, 2021

View reviewed changes

write secrets for new connector

dd4e982

sherifnada reviewed Feb 27, 2021

View reviewed changes

eugene-kulak added 7 commits March 10, 2021 07:38

drop singer connector registration

fc57695

refactor streams, resolve properties in schemas at runtime

a2ee039

replace deprecated endpoint for company contacts

aa766eb

replace deprecated pipeline endpoint

759043e

update comments

4d6c851

update docs

bca6024

fix typo

3b65548

eugene-kulak added 2 commits March 11, 2021 23:49

update schemas with updated field, change engagement layout

3a664eb

fix Campaign stream

37f3acb

keu mentioned this pull request Mar 12, 2021

Source Hubspot: schema should load properties dynamically #2219

Closed

remove custom tests

ad6988f

keu mentioned this pull request Mar 12, 2021

Source Hubspot: all streams should use start_date #2220

Closed

keu requested review from yevhenii-ldv and vitaliizazmic March 12, 2021 08:05

sherifnada reviewed Mar 12, 2021

View reviewed changes

remove dependency

e00c32a

sherifnada approved these changes Mar 12, 2021

View reviewed changes

eugene-kulak and others added 2 commits March 13, 2021 00:45

remove oauth

1312d21

keu linked an issue Mar 19, 2021 that may be closed by this pull request

Source Hubspot: ensure best practices are applied (rate limiting, tests all streams etc.) #2221

Closed

4 tasks

keu removed a link to an issue Mar 19, 2021

Source Hubspot: ensure best practices are applied (rate limiting, tests all streams etc.) #2221

Closed

4 tasks

keu linked an issue Mar 19, 2021 that may be closed by this pull request

Airbyte Native Hubspot connector #2114

Closed

keu changed the title ~~Source Hubspot: native client should support all streams in full sync mode~~ Source Hubspot: native client Mar 19, 2021

Source Hubspot: best practices (#2537)

61c8805

* fix error reporting and add unit tests * fix test and refactor cursor fields * format Co-authored-by: Eugene Kulak <kulak.eugene@gmail.com>

keu changed the title ~~Source Hubspot: native client~~ 🎉 New source: Native Hubspot connector Mar 19, 2021

keu changed the title ~~🎉 New source: Native Hubspot connector~~ 🎉 New source: Native Hubspot connector Mar 19, 2021

keu changed the base branch from ykurochkin/hubspot-create-native-connector-with-schema-folder-populated to master March 19, 2021 22:26

Merge remote-tracking branch 'origin/master' into keu/source-hubspot-…

2764dd3

…native

restored configured_catalog.json

a35389b

keu merged commit 0b9b038 into master Mar 20, 2021

keu deleted the keu/source-hubspot-native branch March 20, 2021 09:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🎉 New source: Native Hubspot connector #2215

🎉 New source: Native Hubspot connector #2215

eugene-kulak commented Feb 25, 2021 •

edited by keu

Loading

eugene-kulak commented Feb 26, 2021 •

edited by github-actions bot

Loading

eugene-kulak commented Feb 26, 2021 •

edited by github-actions bot

Loading

sherifnada left a comment

sherifnada Feb 27, 2021

eugene-kulak Feb 27, 2021

sherifnada Mar 1, 2021

keu Mar 12, 2021

sherifnada Feb 27, 2021

eugene-kulak Feb 27, 2021 •

edited

Loading

sherifnada Feb 27, 2021

sherifnada Feb 27, 2021

sherifnada Feb 27, 2021

sherifnada Feb 27, 2021

sherifnada Feb 27, 2021

sherifnada Feb 27, 2021

sherifnada Feb 27, 2021

sherifnada Feb 27, 2021

keu commented Mar 12, 2021 •

edited by github-actions bot

Loading

sherifnada Mar 12, 2021

sherifnada Mar 12, 2021

keu Mar 12, 2021

keu commented Mar 19, 2021 •

edited by github-actions bot

Loading

keu commented Mar 19, 2021 •

edited by github-actions bot

Loading

keu commented Mar 20, 2021 •

edited by github-actions bot

Loading

keu commented Mar 20, 2021 •

edited by github-actions bot

Loading

keu commented Mar 20, 2021 •

edited by github-actions bot

Loading

		url = "/crm/v3/objects/quotes"


		class TicketsAPI(CRMObjectsAPI):

		@@ -34,5 +50,5 @@ The connector is restricted by normal Hubspot [rate limitations](https://legacyd

		* For api key auth, in Hubspot, for the account to go settings -> integrations \(under the account banner\) -> api key. If you already have an api key you can use that. Otherwise generated a new one.

🎉 New source: Native Hubspot connector #2215

🎉 New source: Native Hubspot connector #2215

Conversation

eugene-kulak commented Feb 25, 2021 • edited by keu Loading

What

How

Pre-merge Checklist

Recommended reading order

eugene-kulak commented Feb 26, 2021 • edited by github-actions bot Loading

eugene-kulak commented Feb 26, 2021 • edited by github-actions bot Loading

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

eugene-kulak Feb 27, 2021 • edited 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

keu commented Mar 12, 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

keu commented Mar 19, 2021 • edited by github-actions bot Loading

keu commented Mar 19, 2021 • edited by github-actions bot Loading

keu commented Mar 20, 2021 • edited by github-actions bot Loading

keu commented Mar 20, 2021 • edited by github-actions bot Loading

keu commented Mar 20, 2021 • edited by github-actions bot Loading

eugene-kulak commented Feb 25, 2021 •

edited by keu

Loading

eugene-kulak commented Feb 26, 2021 •

edited by github-actions bot

Loading

eugene-kulak commented Feb 26, 2021 •

edited by github-actions bot

Loading

eugene-kulak Feb 27, 2021 •

edited

Loading

keu commented Mar 12, 2021 •

edited by github-actions bot

Loading

keu commented Mar 19, 2021 •

edited by github-actions bot

Loading

keu commented Mar 19, 2021 •

edited by github-actions bot

Loading

keu commented Mar 20, 2021 •

edited by github-actions bot

Loading

keu commented Mar 20, 2021 •

edited by github-actions bot

Loading

keu commented Mar 20, 2021 •

edited by github-actions bot

Loading