Removed Stream.bracketCancellable and Pull.acquire* #1603
Merged
Conversation
This file contains hidden or 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
…res in fs2-io, moved FileHandle creation to Resource and in FileHandle companion
SystemFw
approved these changes
Sep 10, 2019
| * Takes the first value output by this stream and returns it in the result of a pull. | ||
| * If no value is output before the stream terminates, the pull is failed with a `NoSuchElementException`. | ||
| * If more than 1 value is output, everything beyond the first is ignored. | ||
| */ |
There was a problem hiding this comment.
I wonder if there is any particular reason for the inconsistency with compile.lastOrError
There was a problem hiding this comment.
What's the inconsistency, just the name? I'll rename to headOrError.
There was a problem hiding this comment.
Just the last vs first behaviour. I can see how Pull is more first-y (uncons), but it's inconsistent with lastOrError. Not a big deal though
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Stream.bracketCancellableandStream.bracketCaseCancellableallowed early, manual finalization of resources. Upon invoking the finalizer, the internal resource would be cleaned up, ensuring we don't leak references to already invoked finalizers. Now that we introduce a scope for eachbracket, folks would expect that invoking cancelation of this case also closes the dedicated scope -- i.e., we don't leak scopes either. Instead of implementing this, I'd like to remove these operations instead. There were no tests for these operations and they were very rarely used. Searching GitHub shows no GH hosted open source projects use either operation.Similarly,
Pull.acquireCancellableprovided a mechanism to manually invoke finalizers. I added this back in #662 to fix resource leaks, but the motivating use cases at the time are all addressed now by using effect level bracketing.Removal of
Pull.acquireis perhaps the most controversial. WhenPullwas the primary mechanism in which scopes were created, acquiring a resource viaPullmade more sense than it does now.This PR also introduces a
s.pull.headOrErrormethod, that converts aStream[F, O]to aPull[F, INothing, O]. Uses ofPull.acquirecan be replaced withStream.bracket(acquire)(release).pull.headOrError.Note: the only place where
Pull.Cancellablewas used was in the constructors forFileHandleinfs2.io.file.pulls. These were vestiges of a time beforecats.effect.Resource. These constructors were removed in favor of new constructors onFileHandlecompanion that returnResource[F, FileHandle[F]].