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

Added associations to some CRM Object streams in Hubspot connector #9027

Closed
wants to merge 2 commits into from

Conversation

ksoenandar
Copy link
Contributor

@ksoenandar ksoenandar commented Dec 22, 2021

What

Very minor change - added company associations to contacts, deals and tickets CRM object streams.

🚨 User Impact 🚨

None, the only change is the instantiation of the streams in the client.py file in the Hubspot source connector.


This change is Reviewable

@github-actions github-actions bot added the area/connectors Connector related issues label Dec 22, 2021
@marcosmarxm marcosmarxm self-assigned this Dec 22, 2021
Copy link
Member

@marcosmarxm marcosmarxm left a comment

Choose a reason for hiding this comment

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

One comment, after is ready to go!

"deal_pipelines": DealPipelineStream(**common_params),
"deals": DealStream(associations=["contacts"], **common_params),
"deals": DealStream(associations=["contacts", "companies"], **common_params),
Copy link
Member

Choose a reason for hiding this comment

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

Adding new associations change the response from the API. You need to change the schema file to be compatible with the new response.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah of course @marcosmarxm!

I've updated the schemas to include the relevant associations. Seems like the deals stream schema has always had the companies associations so I didn't add that.

@marcosmarxm
Copy link
Member

@ksoenandar can you run ./gradlew airbyte-integrations:connectors:source-hubspot:integrationTest looks is still failing.

@ksoenandar
Copy link
Contributor Author

@marcosmarxm - any idea why it was failing? I tried running the integration test but I'm encountering this error: Exception in thread "main" javax.net.ssl.SSLHandshakeException: PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target which seems like it's an unrelated error.

@marcosmarxm
Copy link
Member

@marcosmarxm - any idea why it was failing? I tried running the integration test but I'm encountering this error: Exception in thread "main" javax.net.ssl.SSLHandshakeException: PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target which seems like it's an unrelated error.

This looks a problem with Java, do you have Java 17 installed?

@ksoenandar
Copy link
Contributor Author

@marcosmarxm - Yeah I do actually. Went through a couple of troubleshooting from Stack Overflow and it's not working either. Any other suggestion on how I can run the integration test?

@ksoenandar
Copy link
Contributor Author

Ok @marcosmarxm - managed to solve the Java dependencies issue. I ran that command that you asked for but I'm not sure what I'm looking for. I got the following error log message from that command:

Execution failed for task ':airbyte-integrations:connectors:source-hubspot:airbyteDocker'.
Process 'command '/Users/kevinsoenandar/Documents/airbyte-contribution/airbyte/tools/bin/build_image.sh'' finished with non-zero exit value 1

But not quite sure what this means?

@ksoenandar
Copy link
Contributor Author

Hey @marcosmarxm - update here: I tried forking airbyte's master repo and run the same command for the source hubspot directory as is and I'm encountering the same error message. So doesn't sound like the error is coming from the changes that I made?

@marcosmarxm
Copy link
Member

thanks @ksoenandar I'd published your changes here #10631

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants