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

#384: Add | teragrep exec foreachbatch command that sets processing to batch mode #390

Merged
merged 4 commits into from
Oct 30, 2024

Conversation

eemhu
Copy link
Contributor

@eemhu eemhu commented Oct 24, 2024

Adds | teragrep exec foreachbatch command, which turns off batch collect and sets processing to batch mode via step properties.

See issue

@eemhu eemhu marked this pull request as ready for review October 24, 2024 09:00
@eemhu eemhu requested a review from 51-code October 24, 2024 09:00
Copy link
Contributor

@51-code 51-code left a comment

Choose a reason for hiding this comment

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

I guess this has to be tested in QA mostly as the unit tests don't support batches of data?

@eemhu
Copy link
Contributor Author

eemhu commented Oct 24, 2024

I guess this has to be tested in QA mostly as the unit tests don't support batches of data?

Yes, with the current testing setup it's not really possible. As-is the test only really tests that the command produces expected results data-wise, and with only one batch.

Copy link
Contributor

@51-code 51-code left a comment

Choose a reason for hiding this comment

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

LGTM now, I don't believe testing batches is possible in unit tests. There's an open issue #134 about that.

@eemhu eemhu requested a review from kortemik October 30, 2024 07:54
@kortemik kortemik merged commit 04b3308 into teragrep:main Oct 30, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants