informatikr / hedis

A Redis client library for Haskell.
http://hackage.haskell.org/package/hedis
BSD 3-Clause "New" or "Revised" License
327 stars 121 forks source link

travis: Upgrade to Redis 6.0.9 & Fix auth test #158

Closed matobet closed 3 years ago

matobet commented 3 years ago

Fixes informatikr/hedis#155

matobet commented 3 years ago

There is one problem with the 6.0.9. upgrade though, and that is that the XINFO STREAM command now returns a different order of fields which breaks the order-dependent parsing in ManualCommands.hs

instance RedisResult XInfoStreamResponse where
    decode (MultiBulk (Just [
        Bulk (Just "length"),Integer xinfoStreamLength,
        Bulk (Just "radix-tree-keys"),Integer xinfoStreamRadixTreeKeys,
        Bulk (Just "radix-tree-nodes"),Integer xinfoStreamRadixTreeNodes,
        Bulk (Just "groups"),Integer xinfoStreamNumGroups,
        Bulk (Just "last-generated-id"),Bulk (Just xinfoStreamLastEntryId),
        Bulk (Just "first-entry"), rawFirstEntry ,
        Bulk (Just "last-entry"), rawLastEntry ])) = do
            xinfoStreamFirstEntry <- decode rawFirstEntry
            xinfoStreamLastEntry <- decode rawLastEntry
            return XInfoStreamResponse{..}
    decode a = Left a

The new Redis now returns [ ..., "last-generated-id", "groups", ... ] instead of [ ..., "groups", "last-generated-id", ... ]. I could fix this PR by simply swapping the appropriate lines, but that will probably break all users that use other than the very latest Redis 6.0.9 version.

Ideally those MultiBulk-parsing commands should be reworked to treat the data as order-insensitive hashes to guarantee backwards as well as future compatibility. What do you think @k-bx we should do in this particular case?

matobet commented 3 years ago

One approach would be (if we want to keep the pattern matching syntax on list) to have the entries first sorted by a view pattern by key. Then we would pattern match on the fields in alphabetical order.

k-bx commented 3 years ago

@matobet I suggest we have two functions decodeRedis6XInfoStreamResponse and decodeRedisPre6XInfoStreamResponse, chain them via Alternative/<|>, put a docstring to explain what's happening. Should default to redis 6 format.

matobet commented 3 years ago

Done.

Also, unrelated to this PR, the build for GHC 8.0.1 seem to be failing (also on master):

/home/travis/build/informatikr/hedis/src/Database/Redis/Sentinel.hs:218:29: error:
    • No instance for (Exception (NonEmpty (HostName, PortID)))
        arising from the first field of ‘NoSentinels’
          (type ‘NonEmpty (HostName, PortID)’)
      Possible fix:
        use a standalone 'deriving instance' declaration,
          so you can specify the instance context yourself
    • When deriving the instance for (Exception RedisSentinelException)
--  While building package hedis-0.13.0 using:
      /home/travis/.stack/setup-exe-cache/x86_64-linux/Cabal-simple_mPHDZzAJ_1.24.0.0_ghc-8.0.1 --builddir=.stack-work/dist/x86_64-linux/Cabal-1.24.0.0 build lib:hedis test:doctest test:hedis-test --ghc-options ""
    Process exited with code: ExitFailure 1
The command "stack --no-terminal --skip-ghc-check test" exited with 1.
cache.2
store build cache

I made a fix for that in a separate PR https://github.com/informatikr/hedis/pull/160

k-bx commented 3 years ago

@matobet thank you very much! Released under 0.13.1