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

DV2 TypingDedupingTest: read container stdout in real time #34173

Merged
merged 4 commits into from
Jan 12, 2024

Conversation

edgao
Copy link
Contributor

@edgao edgao commented Jan 11, 2024

discovered accidentally - if a connector container logs too much to stdout, it eventually gets blocked because the buffer fills up. Dump stdout in a background thread instead.

I could be convinced to pass the executor service / future through to the endSync method? but that seemed overkill

@edgao edgao requested a review from a team as a code owner January 11, 2024 16:20
@octavia-squidington-iii octavia-squidington-iii added the CDK Connector Development Kit label Jan 11, 2024
Copy link

vercel bot commented Jan 11, 2024

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

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
airbyte-docs ⬜️ Ignored (Inspect) Visit Preview Jan 12, 2024 0:06am

);
messageHandler.submit(() -> {
while (!destination.isFinished()) {
destination.attemptRead();
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't see another call for attemptRead so seems safe but just a note that attemptRead isn't thread safe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep it's only called from here. I'll add a comment though

@edgao
Copy link
Contributor Author

edgao commented Jan 11, 2024

no need to publish cdk since T+D isn't (yet) in the cdk. No need to publish connectors since this is just test code. merging directly.

@edgao edgao enabled auto-merge (squash) January 11, 2024 21:09
@edgao edgao merged commit a4ff89c into master Jan 12, 2024
17 checks passed
@edgao edgao deleted the edgao/tdtest/performance branch January 12, 2024 00:16
jatinyadav-cc pushed a commit to ollionorg/datapipes-airbyte that referenced this pull request Feb 26, 2024
jatinyadav-cc pushed a commit to ollionorg/datapipes-airbyte that referenced this pull request Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CDK Connector Development Kit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants