Skip to content

Move allowsDynamicSplitting to Reader, and set it in CompressedSource.#502

Closed
peihe wants to merge 2 commits intoapache:masterfrom
peihe:compression
Closed

Move allowsDynamicSplitting to Reader, and set it in CompressedSource.#502
peihe wants to merge 2 commits intoapache:masterfrom
peihe:compression

Conversation

@peihe
Copy link
Contributor

@peihe peihe commented Jun 20, 2016

Before the change in CompressedSource, testUnsplittable() fails in:
assertNull(reader.splitAtFraction(fractionConsumed));

@peihe
Copy link
Contributor Author

peihe commented Jun 20, 2016

R: @dhalperi

// checkpoint every 9 elements
if (actual.size() % 9 == 0) {
Double fractionConsumed = reader.getFractionConsumed();
if (fractionConsumed != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

when can fractionConsumed be null? This test might fail for a variety of reasons.

Copy link
Contributor

Choose a reason for hiding this comment

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

or rather, might silently pass. E.g., if fractionConsumed is always null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@dhalperi
Copy link
Contributor

LGTM, will merge once green.

@asfgit asfgit closed this in e0cae9f Jun 23, 2016
@peihe peihe deleted the compression branch June 28, 2016 20:49
pl04351820 pushed a commit to pl04351820/beam that referenced this pull request Dec 20, 2023
~~Based on branch for PR apache#500 -- I will rebase after that PR merges.~~

Closes apache#367.
    
Supersedes PR apache#497.
Amar3tto added a commit to akvelon/beam that referenced this pull request Mar 4, 2025
Add refresh looker .yml workflow and .py script
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.

2 participants