-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Source-Hubspot - Update Check Steam to Prevent OOM #60985
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
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚢
pending this fix working, i assume we would merge this in after
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
agree with changing stream for check command (see comment).
on other hand it's changing required scope for check, so we may have more config errors on check for some period, when users need to add crm.objects.owners.read
, but I think it's acceptible.
What
OC Issue: https://github.com/airbytehq/oncall/issues/7947
Some streams are OOM'ing (
137
) on CHECK. The current stream used for thiscontacts
requires multiple outbound requests to pass. We will switch it toOwners
, since it requires fewer outbound requests to pass and still verifies that the config provided by the user is good.How
Update manifest:
Also updated read.me to have the Docker commands for Hubspot, since this is now a manifest-only connector, it is useful to have those referenced in the read.me when debugging.
First, we will test via Dev Images. If it passes, we will proceed with the merge. Otherwise, we will consider increasing the resource requirements in addition to this change.
Review guide
User Impact
Can this PR be safely reverted and rolled back?