Skip to content

Migrate IOExceptions to N5IOExceptions for ReadData and related logic - #155

Merged
bogovicj merged 3 commits into
masterfrom
readDataN5IOExceptions
Jun 13, 2025
Merged

Migrate IOExceptions to N5IOExceptions for ReadData and related logic#155
bogovicj merged 3 commits into
masterfrom
readDataN5IOExceptions

Conversation

@cmhulbert

Copy link
Copy Markdown
Contributor

when working with data backed by ReadData, it's not possible to know where the underlying data is coming from. We expect to deal with N5IOExceptions rather than IOException, especially when the ReadData is wrapping a KeyValueAccess read/write. This has the added benefit of letting the ReadData provider wrap the specific IOException with an appropriate N5 specific N5IOException, such as N5NoSuchKeyException.

@cmhulbert
cmhulbert force-pushed the readDataN5IOExceptions branch from e6c68c8 to 91e8284 Compare June 3, 2025 12:46
@cmhulbert
cmhulbert requested a review from tpietzsch June 3, 2025 12:47
@cmhulbert
cmhulbert force-pushed the readDataN5IOExceptions branch from 91e8284 to 19c863e Compare June 3, 2025 12:53
@cmhulbert

Copy link
Copy Markdown
Contributor Author

Generally happy with this change. I think we should consider even more what to do with IOExceptions, especially in KeyValueAccess implementations. Specifically, LockedChannel still throws IOExceptions, and we may want to wrap those as well. Not necessarily for this PR though

@cmhulbert

Copy link
Copy Markdown
Contributor Author

Generally I tried to remove any IOExceptions in favore of N5IOExceptions for all interface methods, especially for KeyValueAccess. Implementations are still welcome to throw IOExceptions where appropriate internally. but should wrap IOExceptions to N5IOExceptions before throwing in public API methods.

@bogovicj
bogovicj merged commit f227ffa into master Jun 13, 2025
2 checks passed
@cmhulbert
cmhulbert deleted the readDataN5IOExceptions branch November 5, 2025 21:13
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