Skip to content

feat: Deprecate custom NotFoundError in favor of built-in FileNotFoundError #487

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

Merged
merged 2 commits into from
Jun 25, 2025

Conversation

kylebarron
Copy link
Member

@kylebarron kylebarron commented Jun 23, 2025

Custom exceptions with multiple inheritance are not currently possible in pyo3 (at least to define on the Rust side). See PyO3/pyo3#4979

You could have a custom exception with multiple inheritance if you defined it on the Python side and then imported it into Rust... but then that would leak into pyo3-object_store and I want that to be self contained and not require other users of pyo3-object_store to have a dependency on some Python package where the exception is defined.

So I think the best way forward for now is to choose one or the other. Since we're already using FileNotFoundError, let's just use that one.

The primary downside is that obstore.exceptions.BaseError is not universally the base of everything we raise.

Closes #197, closes #199

@github-actions github-actions bot added the feat label Jun 23, 2025
@lukasbindreiter
Copy link

The only small downside with fully using builtins.FileNotFoundError I think is it's name - since no files are necessarily involved.

from obstore.store import MemoryStore

MemoryStore().get("non-existent")  # FileNotFoundError
# no files involved anywhere

I guess that's why the exception in obstore was just named NotFoundError?
For the above example KeyError would probably be the more appropriate, but then for other stores not really.

@kylebarron
Copy link
Member Author

All stores will raise the same exceptions, that's for sure. Having different stores raise different exceptions would be so confusing IMO

@kylebarron kylebarron merged commit 60146b7 into main Jun 25, 2025
8 checks passed
@kylebarron kylebarron deleted the kyle/deprecate-notfounderror branch June 25, 2025 05:14
@lukasbindreiter
Copy link

Having different stores raise different exceptions would be so confusing IMO

yeah for sure, totally agree with that. Just wanted to mention that FileNotFoundError itself may also be confusing in certain scenarios - but for now it is the best approach for sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NotFoundError is not raised, but Python FileNotFoundError is raised
2 participants