-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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 Klaviyo : Migrate to Manifest-only #54166
Conversation
…pe/klaviyo/migrate-manifest-only
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
/format-fix
|
CATs passed, only unit tests fail due to unresolved dependency issue (requests-mock) |
@darynaishchenko can you please give this a review and a quick spin? It looks like we should consider this AFTER we merge the API update, right? |
@topefolorunso could you please fix failed unit tests? |
@topefolorunso , overall LGTM, but can you fix unit tests and merge conflicts so we can run regression tests? Thanks! |
…pe/klaviyo/migrate-manifest-only
@darynaishchenko The failing unit test is due to missing request-mock package in the ci environment. All tests pass in local |
@topefolorunso, you can try to add requests-mock dependency to |
didn't work @darynaishchenko, got the same error |
Regression tests:Only one failure: Stream events has 7 more records in the target version (2560 vs. 2553). |
/approve-regression-tests
|
@natikgadzhi this looks good, can we merge it today? |
Done! |
Closes https://github.com/airbytehq/airbyte-internal-issues/issues/11684