Skip to content

[BEAM-2708] Adds support for reading concatenated bzip2 files#3678

Closed
chamikaramj wants to merge 2 commits intoapache:masterfrom
chamikaramj:pbzip2_test
Closed

[BEAM-2708] Adds support for reading concatenated bzip2 files#3678
chamikaramj wants to merge 2 commits intoapache:masterfrom
chamikaramj:pbzip2_test

Conversation

@chamikaramj
Copy link
Contributor

Adds tests for concatenated gzip and bzip2 files.

Removes test 'test_model_textio_gzip_concatenated' in 'snippets_test.py' since it's actually hitting 'DummyReadTransform' and not testing this feature.

Adds tests for concatenated gzip and bzip2 files.

Removes test 'test_model_textio_gzip_concatenated' in 'snippets_test.py' since it's actually hitting 'DummyReadTransform' and not testing this feature.
@chamikaramj
Copy link
Contributor Author

R: @aaltay

cc: @robertwb

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.007%) to 70.158% when pulling 40e1fbf on chamikaramj:pbzip2_test into de0148c on apache:master.

Copy link
Member

@aaltay aaltay left a comment

Choose a reason for hiding this comment

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

LGTM after fixing the minor comments. Thank you.

Please consider CPing this to the release branch.

pipeline.run()

def test_read_gzip_concat(self):
# gzip_file_name1 = temp_path_1 + '.gz'
Copy link
Member

Choose a reason for hiding this comment

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

Remove commented code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

self._read_buffer.write(decompressed)
continue
else:
# Gzip and bzip2 formats do not require flusing remaining data in the
Copy link
Member

Choose a reason for hiding this comment

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

flusing -> flushing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@chamikaramj
Copy link
Contributor Author

Thanks.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 70.166% when pulling bdfec5a on chamikaramj:pbzip2_test into de0148c on apache:master.

@asfgit asfgit closed this in d4f9e92 Aug 3, 2017
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