taoensso / nippy

The fastest serialization library for Clojure
https://www.taoensso.com/nippy
Eclipse Public License 1.0
1.04k stars 60 forks source link

{... :skip-header? true} breaks thaw #73

Closed chairmanwow closed 8 years ago

chairmanwow commented 8 years ago

This is for 2.11.0-alpha5.


(def nippy-opts {:compressor lz4-compressor :encryptor nil :skip-header? true})

(thaw (freeze :foo nippy-opts) nippy-opts)

"Thaw failed: Decryption/decompression failure, or data unfrozen/damaged." [taoensso.nippy$thaw$ex364 invoke "nippy.clj" 695] [taoensso.nippy$thaw$ex364 invoke "nippy.clj" 694]

I'd try to be more helpful, but the line numbers don't match what's in the repo.

chairmanwow commented 8 years ago

Sweet, it was just in the dev branch. May want to add the (or ...) like you have in freeze (nippy.clj:411) to thaw (nippy.clj:692). Didn't see that the param changed to :no-header? .

Should we expect that :skip-header? is getting deprecated in favor of :no-header? ?

Thanks,

ptaoussanis commented 8 years ago

Hi Becker,

The relevant change here is that thaw now requires a :no-header? true option to match freeze's :no-header? true. Previously, thaw would try auto-detect when the header was absent. The new approach is faster and more hygienic - but does require that you know in advance which payloads were frozen w/o a header.

Does that help / make sense?

ptaoussanis commented 8 years ago

Oh, as an aside: please note that any payload large enough to warrant compression should very likely have a header - so I'd go with either:

(def nippy-opts {:compressor nippy/lz4-compressor :encryptor nil})
(nippy/thaw (nippy/freeze :foo nippy-opts) nippy-opts)

;; or

(def nippy-opts {:compressor nil :encryptor nil :no-header? true})
(nippy/thaw (nippy/freeze :foo nippy-opts) nippy-opts)
chairmanwow commented 8 years ago

Should've weighed the costs a bit more before going ahead with that -- good feedback.