Skip to content

Commit

Permalink
refactor: expect writer to be closed when creating manifest files
Browse files Browse the repository at this point in the history
  • Loading branch information
felixscherz committed Jul 9, 2024
1 parent ffbc160 commit e1893a0
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 8 deletions.
6 changes: 3 additions & 3 deletions pyiceberg/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ def __exit__(
traceback: Optional[TracebackType],
) -> None:
self._close_current_writer()
self.closed = True
self._closed = True

def _get_current_writer(self) -> ManifestWriter:
if self._should_roll_to_new_file():
Expand All @@ -833,8 +833,8 @@ def _close_current_writer(self):
self._current_file_rows = 0

def to_manifest_files(self) -> list[ManifestFile]:
self._close_current_writer()
self._closed = True
if not self._closed:
raise RuntimeError("Cannot create manifest files from unclosed writer")
return self._manifest_files

def add_entry(self, entry: ManifestEntry) -> RollingManifestWriter:
Expand Down
13 changes: 8 additions & 5 deletions tests/utils/test_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -539,11 +539,12 @@ def supplier() -> Generator[ManifestWriter, None, None]:
) as writer:
for entry in manifest_entries:
writer.add_entry(entry)
manifest_files = writer.to_manifest_files()
assert len(manifest_files) == expected_number_of_files
with pytest.raises(RuntimeError):
# It is already closed
writer.add_entry(manifest_entries[0])

manifest_files = writer.to_manifest_files()
assert len(manifest_files) == expected_number_of_files
with pytest.raises(RuntimeError):
# It is already closed
writer.add_entry(manifest_entries[0])


@pytest.mark.parametrize("format_version", [1, 2])
Expand Down Expand Up @@ -613,3 +614,5 @@ def test_write_manifest_list(
assert entry.file_sequence_number == 0 if format_version == 1 else 3
assert entry.snapshot_id == 8744736658442914487
assert entry.status == ManifestEntryStatus.ADDED


0 comments on commit e1893a0

Please sign in to comment.