Skip to content

fix(python): raise instead of panicking in the manifest bindings - #3241

Open
rambleraptor wants to merge 1 commit into
apache:mainfrom
rambleraptor:fix-python-binding-panics
Open

rambleraptor wants to merge 1 commit into
apache:mainfrom
rambleraptor:fix-python-binding-panics

Conversation

@rambleraptor

@rambleraptor rambleraptor commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

What changes are included in this PR?

This changes a handful of Python functions to return PyResult, which allows us to propagate errors up.

It also changes the type signature of PyManifest.partitions() to allow None.

Neither of these breaks the current type signatures, although the partitions() change may result in a Python loop throwing a ValueError.

Are these changes tested?

Included unit tests. Pretty basic test plan:

  • Make sure that read_manifest_entries throws an error when given bad input
  • Make sure that read_manifest_list throws an error when given bad input
  • Make sure that .partitions() can successfully return None

AI Disclosure

Claude built this PR out under my direct supervision and questioning. I've been doing a lot of Rust investigation as part of my work on PyIceberg. The community has had a lot of great conversations around AI! I haven't sent many PRs to iceberg-rust and hoping that these clear the bar.

Claude had no part in writing my PR description or (hastily written) GitHub issue.

There's some assorted places that we need to raise errors instead
of panicking.
@rambleraptor

Copy link
Copy Markdown
Contributor Author

@kevinjqliu fyi since you've done a lot of work on pyiceberg-core

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.

Reduce panics in pyiceberg_core

1 participant