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 Outreach: Remove custom incremental class and use new filter syntax #55805

Closed
wants to merge 5 commits into from

Conversation

kyleromines
Copy link
Contributor

@kyleromines kyleromines commented Mar 17, 2025

What

Previous PR here started failing due to cdk updates: #55180
It turns out Outreach has a new filter syntax, docs here: https://developers.outreach.io/api/making-requests/#new-filter-syntax

This allows us to add the the greater than or equal logic to the request parameter name instead of in the paramater value.
Now requests look like .com/accounts?filter[updatedAt][gte]=2017-01-01 instead of .com/accounts?filter[updatedAt]=2017-01-01..inf

This eliminates the need for the custom class or complicated injection logic in the yaml.

Also changed datetime_format to %_ms instead of %f as the api returns milliseconds and %f was causing the start date timestamp to get converted to microseconds which will error out the api request as it only goes up to miliseconds.

How

Add parameter newFilterSyntax = true, remove CustomIncrementalSync class and switch to start_time_option with field_name = filter[updatedAt][gte].

Review guide

  1. manifest.yaml
  2. components.py

User Impact

Currently cannot set up source with latest version as the check fails and upgrading an existing source to the latest connector version causes syncs to fail with same error.

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

Copy link

vercel bot commented Mar 17, 2025

@kyleromines is attempting to deploy a commit to the Airbyte Growth Team on Vercel.

A member of the Team first needs to authorize it.

@kyleromines kyleromines changed the title Remove custom incremental class 🐛 Source Outreach: Remove custom incremental class Mar 17, 2025
@kyleromines kyleromines changed the title 🐛 Source Outreach: Remove custom incremental class 🐛 Source Outreach: Remove custom incremental class and use new filter syntax Mar 17, 2025
@kyleromines
Copy link
Contributor Author

kyleromines commented Mar 17, 2025

@natikgadzhi @pnilan @btkcodedev Got this one updated should be good to go now.

@shubham-agrawal-glean
Copy link

@kyleromines when we can merge this? I am also getting below error:
if self.cursor_field.string in stream_state: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TypeError: argument of type 'NoneType' is not iterable

@kyleromines
Copy link
Contributor Author

@shubham-agrawal-glean I cannot merge as I am not a member of the Airbyte team. @natikgadzhi has been helping me on the other PRs.

@shubham-agrawal-glean
Copy link

Thanks @kyleromines for update. @natikgadzhi Could you please help with this on urgent basis, I am exploring airbyte to get data from outreach and if this works, it will save a lot of time writting code to get data via API.

@marcosmarxm
Copy link
Member

You're submitting a change directly from your fork master branch. It won't possible to run integration tests and validate your changes. Please submit a new contribution creating a new branch. More info here.

@kyleromines
Copy link
Contributor Author

@marcosmarxm Silly...
I opened a new one here #55888

@shubham-agrawal-glean
Copy link

@kyleromines Thanks for quick response.
@marcosmarxm Can we approve and merge the new request? #55888

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants