✨ Source Freshcaller : Migrate to Manifest-only

[topefolorunso]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 14, 2025 0:13am

[topefolorunso]

/format-fix

Format-fix job started... Check job output.

✅ Changes applied successfully. (1630219)

[natikgadzhi]

The CI says that incremental reads are broken and always return the same records. Probably filtering by updated at is broken.

[natikgadzhi]

16 new failing acceptance tests detected:
-test_core.py::TestDiscovery::test_defined_refs_exist_in_schema[inputs0]
-test_core.py::TestDiscovery::test_defined_keyword_exist_in_schema[inputs0-not]
-test_core.py::TestDiscovery::test_streams_has_sync_modes[inputs0]
-test_full_refresh.py::TestFullRefresh::test_sequential_reads[inputs0]
-test_core.py::TestDiscovery::test_backward_compatibility[inputs0]
-test_core.py::TestBasicRead::test_read[inputs0]
-test_incremental.py::TestIncremental::test_state_with_abnormally_large_values[inputs0]
-test_core.py::TestDiscovery::test_primary_keys_exist_in_schema[inputs0]
-test_incremental.py::TestIncremental::test_two_sequential_reads[inputs0]
-test_core.py::TestDiscovery::test_defined_keyword_exist_in_schema[inputs0-allOf]
-test_core.py::TestDiscovery::test_primary_keys_data_type[inputs0]
-test_incremental.py::TestIncremental::test_read_sequential_slices[inputs0]
-test_core.py::TestDiscovery::test_defined_cursors_exist_in_schema[inputs0]
-test_core.py::TestDiscovery::test_streams_have_valid_json_schemas[inputs0]
-test_core.py::TestConnection::test_check[inputs1]
-test_core.py::TestDiscovery::test_additional_properties_is_true[inputs0]
-test_core.py::TestConnection::test_check[inputs0]
-test_core.py::TestDiscovery::test_discover[inputs0]
Please fix the new failing tests before merging this PR.

@topefolorunso nope, that backfired.

[topefolorunso]

The CI says that incremental reads are broken and always return the same records. Probably filtering by updated at is broken.

@natikgadzhi see your comment here #40618 (comment). Could something similar be happening here?

[ChristoGrab]

EDIT: Originally thought the failure might be cased by the test stream only returning a single record. However, I ran regression tests and it does seem like incremental sync breaks with this update, as they show a difference of ~11000 to 30 records between target and control versions:

https://github.com/airbytehq/airbyte/actions/runs/11412961965/job/31759820864

So my current guess is that something about incremental changed in the CDK and it's messing up the existing incremental configuration for this connector when we migrate to the new version.

[natikgadzhi]

cleanup the conflicts, let's merge this

[topefolorunso]

/format-fix

Format-fix job started... Check job output.

✅ Changes applied successfully. (40abd50)

[topefolorunso]

@natikgadzhi Update: Time Travel in Incremental Sync

The failing incremental test is due to time zone correction of the datetime parameter.

The record pulled in the first sequential read has created_time (cursor_field) in -0700 but the cursor_value is stored in +0000. Hence, during the second sequential read, the datetime parameter is set to 7 hours into the past i.e using the same cursor_value but stored in (not converted to) +0000 which makes the api return the same record naively.

Temporary fix: Maintained the time zone received from the api when storing the state and cursor_value. Same time zone is used when making subsequent requests with the state. But what happens if there are different records returned with different time zones?

More permanent fix would be to find a way to correct all incoming datetime from records to +0000 before storing them as state values

[topefolorunso]

Also worthy of note in this PR - I removed the incremental sync for calls stream because the endpoint is not configured properly for time filtering with same parameters as calls_metrics.

[natikgadzhi]

It's really inspiring to see such a deep dive into failure reasons. Thank you @topefolorunso!

[topefolorunso]

@natikgadzhi How do we proceed with this?

[natikgadzhi]

Alright, so — everything related to calls should stay incremental if it was incremental before. The reason for this is the quality of the experience for our users — you'd assume they have a LOT of calls, and you don't want to increase sync time linearly with data volume growth.

@brianjlai can you take a look and advice @topefolorunso on how to go about the cursor? If you have time for a loom, that would prob help us level up Tope so he can figure out situations like this in the future.

[topefolorunso]

/format-fix

Format-fix job started... Check job output.

✅ Changes applied successfully. (75c145e)

[natikgadzhi]

Ideally cursor datetime timezone conversions should be automatic too =/

[topefolorunso]

/format-fix

Format-fix job started... Check job output.

✅ Changes applied successfully. (49d7c63)

[topefolorunso]

@natikgadzhi All green here - did a little improvisation with custom components. You should check it out and confirm it's a safe way to go.