ZIO resource management built on the core API. #230
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jczuchnowski
previously approved these changes
Jul 12, 2020
quelgar
force-pushed
the
quelgar/api-cleanup
branch
from
8357711
to
a92a948
Compare
quelgar
force-pushed
the
quelgar/managed-close
branch
from
df5b2c9
to
ddd7044
Compare
* Change most APIs to use `IOException` as the error type instead of `Exception`. Non-IO exceptions are now considered defects as they only occur due to misuse of the API. * Similarly change incorrect `Buffer` usage errors to be defects instead of failures. * The `Async*` classes still use `Exception` for the error type for now, as the NIO Javadocs are vague as to what kind of exceptions to expect from these. * Rename various read and write methods so they match the underlying NIO methods. Seems clearer. * Add various Scaladocs (more still needed). * Use `Option` to indicate end-of-stream when reading, instead of `-1` magic value.
quelgar
force-pushed
the
quelgar/api-cleanup
branch
from
a92a948
to
018a675
Compare
quelgar
force-pushed
the
quelgar/managed-close
branch
from
ddd7044
to
7453b6d
Compare
End-of-stream is now handled in a consistent way across all read APIs. If a read encounters end-of-stream, the effect will fail with a `java.io.EOFException`.
All closeable or releasable NIO resources now extend `IOCloseable`. Extension methods are provided by `CloseableResourceOps` for convenient access to ZIO bracketing or `ZManaged`.
quelgar
force-pushed
the
quelgar/managed-close
branch
from
7453b6d
to
6cd5737
Compare
quelgar
force-pushed
the
quelgar/api-cleanup
branch
from
81e77c4
to
a303c28
Compare
|
@quelgar There are a lot of changes here, and I heartily approve of most of them. Can you fix the conflicts and we can get this merged in? |
|
@jdegoes I put this up as a draft just to see what people thought of this approach to resource management, but I haven't kept it up to date. It looks like most of the changes showing here are actually from #228, which this was originally based on. Sorry for the confusion! #228 has a description of the API cleanup changes, and no conflicts. I can make a new clean PR focused on just resource management after that's in. |
|
I'll do a new PR that applies cleanly to master soon. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a proposal, based on the observation that the only significant difference between the core module and the "high level" module is that core exposes the low-level
closemethods whereas the high level module offers aZManagedAPI. The vast majority of code in the high level module is an exact duplicate of code in core.This PR attempts to offer the low-level NIO
closemethod—adhering to our unopinionated goal—while allowing the user to opt-in to a bracketed orZManagedAPI instead. Any closeable resource from core can be used via bracketing/ZManagedwithout needing the duplication into a higher level module:If we were to adopt this approach, we'd follow up by removing the
/niomodule and renaming/nio-coreto/nio.