search

ozataman / csv-conduit

Flexible, fast and constant-space CSV library for Haskell using conduits
Other
52 stars 33 forks source link

Update for new conduit, resourceT #33

Closed MichaelXavier closed 6 years ago

MichaelXavier commented 6 years ago

This is for #32

A few things here:

  1. Several operators and type aliases in conduit that have been provided for backwards compatibility are now actually deprecated. I've updated our use of type aliases and operators to the recommended path.
  2. Conduit has dropped the mmorph dependency which means the hoisting we were doing wasn't working. They aliased ExceptionT to CatchT, a monad transformer built for providing MonadThrow for pure code which we need because the CSV machinery requires a MonadThrow instance. CatchT was kind of limited so I handle it just in the CSV parsing part, convert it to an either and then change the transformer stack to ExceptT so it aborts the pipeline if it throws.

However, I think a lot of this is all hypothetical. Looking at the code today, I cannot see a situation where the parsing can even throw an exception. csv-conduit takes your parser, runs it in a non-throwing manner and then eats rows. See #24 for some efforts of fixing that. So I couldn't write a test that exercised this case but the types check and non-exceptional decoding appears to work correctly.

MichaelXavier commented 6 years ago

@ozataman let me know if you have any general opinions on this. I don't expect you to dive deep into what's happening with conduit/resourcet but the short summary:

  1. Latest conduit and resourcet releases have deprecated many previous aliases so I had to update the conduit style so to speak. This sets a higher lower bound on conduit but it was released 2 years ago.
  2. A trick we were doing to catch pure exceptions didn't work anymore. I had to rearrange things but I'm also pretty sure an exception in that context is impossible because csv-conduit eats errors but we're still forced to deal with the case.
  3. It'd probably be a good idea to pick up work again on variants of csv-conduit which don't eat errors but it'd be a good idea to get this out as a separate release because these dependency changes will probably hit stackage nightlies soon.