Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🎉Source Hubspot: Add contacts associations to Deals stream. #5693

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"sourceDefinitionId": "36c891d9-4bd9-43ac-bad2-10e12756272c",
"name": "Hubspot",
"dockerRepository": "airbyte/source-hubspot",
"dockerImageTag": "0.1.10",
"dockerImageTag": "0.1.11",
"documentationUrl": "https://docs.airbyte.io/integrations/sources/hubspot",
"icon": "hubspot.svg"
}
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@
- sourceDefinitionId: 36c891d9-4bd9-43ac-bad2-10e12756272c
name: Hubspot
dockerRepository: airbyte/source-hubspot
dockerImageTag: 0.1.10
dockerImageTag: 0.1.11
documentationUrl: https://docs.airbyte.io/integrations/sources/hubspot
icon: hubspot.svg
- sourceDefinitionId: 95e8cffd-b8c4-4039-968e-d32fb4a69bde
Expand Down
2 changes: 1 addition & 1 deletion airbyte-integrations/connectors/source-hubspot/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ RUN pip install .

ENV AIRBYTE_ENTRYPOINT "/airbyte/base.sh"

LABEL io.airbyte.version=0.1.10
LABEL io.airbyte.version=0.1.11
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to bump to 0.1.12 also get latest code from master because yesterday was merged a change to Hubspot connector.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vladimir-remar please bump the versions in all required places, to the 0.1.12

LABEL io.airbyte.name=airbyte/source-hubspot
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,63 @@ def list(self, fields) -> Iterable:
yield record


class CRMAssociationStream(Stream):
""" see https://developers.hubspot.com/docs/api/crm/associations for more details
"""

entity: Optional[str] = None
associations: List[str] = []
updated_at_field = None
created_at_field = None

@property
def url(self):
"""Entity URL"""
return f"/crm/v3/associations/{self._relationship_from}/{self._relationship_to}/{self._endpoint}"

def __init__(self, relationship_from: str = None, relationship_to: str = None, endpoint: str = None, **kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend the following ordering convention:

  • properties
  • init
  • abstract methods
  • concrete methods

super().__init__(**kwargs)
self._relationship_from = relationship_from
self._relationship_to = relationship_to
self._endpoint = endpoint

def _transform(self, records: Iterable) -> Iterable:
"""Preprocess record """
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add more description in docstring, why we need this method just passing the records.

for record in records:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yield from records in this case, no need for the loop.

yield record

def _filter_old_records(self, records: Iterable) -> Iterable:
"""Skip """
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as comment above.

for record in records:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you can simply:
yield from records in this case, no need for the loop.

yield record


class DealToContactAssociationsStream(CRMAssociationStream):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is useful part of the connector, however I have a question around reusing the parts:

  • we have class CRMObjectStream(Stream) which should provide the necessary functionality for the flat associations for other HubSpot objects, have you tried to reuse it for your stream? Were there any problems of using that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @bazarnov, thanks for the comment and the advices, you are right the class CRMObjectStream(Stream) work more than fine, it should be something like this:

class DealToContactAssociationsStream(CRMObjectStream):
  
  entity = "deal"
  associations = ["contacts"]

Now my question is, how to do a right json schema, since it's almost the same as the deals schema, is it good to duplicate it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can always go with

$ref: schema.json

Inside of your_stream_schema.json
But if you want you can duplicate it, not a big deal.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @bazarnov, thanks for the comment and the advices, you are right the class CRMObjectStream(Stream) work more than fine, it should be something like this:

class DealToContactAssociationsStream(CRMObjectStream):

  

  entity = "deal"

  associations = ["contacts"]

Now my question is, how to do a right json schema, since it's almost the same as the deals schema, is it good to duplicate it?

Sounds great, can wee proceed with reusing those parts, instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'm gonna update the pr with the new changes.

def __init__(self, deal_stream, **kwargs):
super().__init__(relationship_from="Deals", relationship_to="Contacts", endpoint="batch/read", **kwargs)
self._deal_stream = deal_stream

def prepare_payload(self, ids: Iterable) -> Mapping[str, Any]:
payload = {"inputs": []}

for deal_id in ids:
payload['inputs'].append({"id": deal_id})

return payload

def list(self, fields) -> Iterable:

deals = self._deal_stream.list(fields)
ids = []

for deal in deals:
ids.append(deal.get('id'))
if len(ids) == self.limit:
payload = self.prepare_payload(ids=ids)
yield from self.read(partial(self._api.post, url=self.url, data=payload))
ids = []


class DealPipelineStream(Stream):
"""Deal pipelines, API v1,
This endpoint requires the contacts scope the tickets scope.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
OwnerStream,
SubscriptionChangeStream,
WorkflowStream,
DealToContactAssociationsStream,
)


Expand Down Expand Up @@ -69,6 +70,7 @@ def __init__(self, start_date, credentials, **kwargs):
"subscription_changes": SubscriptionChangeStream(**common_params),
"tickets": CRMObjectStream(entity="ticket", **common_params),
"workflows": WorkflowStream(**common_params),
"deal_to_contact_associations": DealToContactAssociationsStream(deal_stream=DealStream(**common_params), **common_params),
}

super().__init__(**kwargs)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"$schema": "http://json-schema.org/draft-07/schema#",
"type": ["null", "object"],
"properties": {
"from": {
"type": ["null", "Object"],
"properties": {
"id": {
"type": ["null", "string"]
}
}
},
"to": {
"type": ["null", "array"],
"items": {
"type": ["null", "Object"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"type": ["null", "Object"],
"type": ["null", "object"],

"properties":{
"id": ["null", "string"],
"type": ["null", "string"]
}
}
}
}
}