Skip to content
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

[Discussion] Should we bring back exceptions instances? #52

Open
ivan-m opened this issue May 21, 2018 · 6 comments
Open

[Discussion] Should we bring back exceptions instances? #52

ivan-m opened this issue May 21, 2018 · 6 comments

Comments

@ivan-m
Copy link
Contributor

ivan-m commented May 21, 2018

I'm not sure of the safety, etc. but since we've removed the ResourceT support it means we can't easily use S.copy to write a Stream to multiple files (as streaming-with needs a MonadMask instance on the Monad for bracket to work).

@mbwgh
Copy link

mbwgh commented Jun 22, 2018

I just opened an issue here which might be related - maybe I should have brought it up here instead.

Regarding MonadMask, when you read the introduction to streaming-with, the fact that there isn't an instance for Stream comes as a surprise.

All in all, as a newcomer to these libraries, regarding resource management I feel kind of lost right now. Class instances were removed without a deprecation period. The API docs still refer to missing functionality. A MonadResource instance exists for MonadResource m => ByteString m. Should I expect this one to vanish as well?

Although this might in part be a documentation issue, the impression I get from this is that the API is still very unstable. Which would be fine of course, but is this a correct assessment?

@ivan-m
Copy link
Contributor Author

ivan-m commented Jun 24, 2018

The problem with the instances is that MonadCatch doesn't work well with continuation-passing-style code, which the Stream type is. As such, as helpful as the instances are, they're going to be wrong (ResourceT was previously used to try and maintain the semantics of MonadCatch but was not successful).

Off the top of my head, I wonder if it's possible/legal to have Stream f m r -> m r be an instance of MonadMask...

@ocharles
Copy link
Contributor

ocharles commented Jul 1, 2018

Why doesn't MonadCatch work with CPS code?

@danidiaz
Copy link
Contributor

danidiaz commented Jul 1, 2018

but since we've removed the ResourceT support it means we can't easily use S.copy to write a Stream to multiple files

Could you detail this a bit more? What would be wrong with using nested withFiles to write each of the layers resulting from copy?

@danidiaz
Copy link
Contributor

The CHANGELOG mentions the problem with ResourceT:

Remove bracketStream, MonadCatch instance, and everything dealing with ResourceT. All of these things of sort of broken for Stream since there is no guarantee of linear consumption (functions like take can prevent finalizers from running).

I guess this means that if you bracketStream access to a file, keep only the first ten lines using take, concat with another, perhaps very large, stream, and then consume the concatenated result, the original file handle won't be closed until we finish with the whole stream and "exit" ResourceT.

About the take problem: if resource management were performed by a decorator above Stream, instead of a monadic layer below it, perhaps we could ensure prompt finalization even in the face of take by carefully handling the stream functions that are "lifted" to the decorator.

I have a proof-of-concept here https://github.com/danidiaz/streaming-bracketed/tree/0.0.0.0 although I'm still a bit unsure about the viability of this approach. And it only works for IO-based Streams.

@chessai
Copy link
Member

chessai commented Jul 17, 2018

@danidiaz, yeah, using something like S.take 100 $ <some file stream operation>, you can leak file descriptors

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

No branches or pull requests

5 participants