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.

Sign up for GitHub

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

Jump to bottom

Bulk load CDK: only microbatch in truncate tests #55178

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Bulk load CDK: only microbatch in truncate tests #55178

wants to merge 2 commits into from
+12 −4

Conversation

edgao
Copy link
Contributor

@edgao edgao commented Mar 3, 2025

as title. from https://airbytehq-team.slack.com/archives/C03AS1GAQV6/p1740779904185839:

Johnny Schmidt
FYI: I took a stab at ripping out the microbatching hack for destination ITs. It was trivial to fix. Tests no longer hang, but they end up running way longer by default (100x), because they're filling up a whole production-sized load before acking. Since batch sizes are dest-specific, this means every connector dev would have to do extra work to set test batch sizes, which is probably more work than occasionally tripping over a microbatch edge case. So I'm going to punt on this for now

Edward Gao
makes sense - I might try only enabling microbatching in the tests that depend on it, which probably alleviates some risk that microbatches mask actual problems

@edgao edgao requested a review from a team as a code owner March 3, 2025 20:03
@vercel Vercel
Copy link

vercel bot commented Mar 3, 2025 •
edited
Loading

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 3, 2025 8:13pm

@octavia-squidington-iii octavia-squidington-iii added the CDK Connector Development Kit label Mar 3, 2025
johnny-schmidt
johnny-schmidt approved these changes Mar 3, 2025
View reviewed changes
Copy link
Contributor

@johnny-schmidt johnny-schmidt left a comment

Choose a reason for hiding this comment

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

I'm curious to see what this breaks 🙃

@johnny-schmidt
Copy link
Contributor

I'm curious to see what this breaks 🙃

FYI I unpinned S3 and ran its tests against this and they all passed.

@edgao
Copy link
Contributor Author

edgao commented Mar 3, 2025

lol. yeah I ran s3 + s3-data-lake locally and things seemed fine 🤞

@edgao edgao enabled auto-merge (squash) March 3, 2025 23:38
@edgao
Copy link
Contributor Author

edgao commented Mar 4, 2025

ugh, why is gradle check consistently timing out

this is going to be something stupid like "the mock integration tests are getting stuck for some dumb reason"

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

@johnny-schmidt johnny-schmidt johnny-schmidt approved these changes

@frifriSF59 frifriSF59 Awaiting requested review from frifriSF59

Assignees
No one assigned
Labels
CDK Connector Development Kit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@edgao @johnny-schmidt @octavia-squidington-iii