Support bloodhound 0.13 in log-elasticsearch

[23Skidoo]

There's a new bloodhound release out, which splits the module hierarchy in V1 and V5 namespaces. We should add support for this to log-elasticsearch, probably by doing a similar split in our module structure.

[phadej]

Is it ok to drop support for bloodhound <0.13, supporting both would mean some ugly CPP

[phadej]

I'd avoid duplicating code and do:

{-# LANGUAGE StandaloneDeriving #-}
module Log.Backend.ElasticSearch.Internal where

import Prelude
import qualified Data.Text as T

import qualified Database.V1.Bloodhound as V1
import qualified Database.V5.Bloodhound as V5

class EsVersion ver where
  type EsUsername ver
  type EsPassword ver

data EsVersion1
data EsVersion5

instance EsVersion EsVersion1 where
  type EsUsername EsVersion1 = V1.EsUsername
  type EsPassword EsVersion1 = V1.EsPassword

instance EsVersion EsVersion5 where
  type EsUsername EsVersion5 = V5.EsUsername
  type EsPassword EsVersion5 = V5.EsPassword

-- | Configuration for the Elasticsearch 'Logger'. See
-- <https://www.elastic.co/guide/en/elasticsearch/reference/current/glossary.html>
-- for the explanation of terms.
data ElasticSearchConfig ver = ElasticSearchConfig {
    esServer        :: !T.Text -- ^ Elasticsearch server address.
  , esIndex         :: !T.Text -- ^ Elasticsearch index name.
  , esMapping       :: !T.Text -- ^ Elasticsearch mapping name.
  , esLogin         :: Maybe (EsUsername ver, EsPassword ver) -- ^ Elasticsearch basic authentication username and password.
  , esLoginInsecure :: !Bool   -- ^ Allow basic authentication over non-TLS connections.
  }

deriving instance (Eq (EsUsername ver), Eq (EsPassword ver)) => Show (ElasticSearchConfig ver)
deriving instance (Show (EsUsername ver), Show (EsPassword ver)) => Show (ElasticSearchConfig ver)

-- | Sensible defaults for 'ElasticSearchConfig'.
defaultElasticSearchConfig :: ElasticSearchConfig ver
defaultElasticSearchConfig = ElasticSearchConfig {
  esServer        = "http://localhost:9200",
  esIndex         = "logs",
  esMapping       = "log",
  esLogin         = Nothing,
  esLoginInsecure = False
  }

I'm not sure how different apis are, but I hope the class can unify the differences

[23Skidoo]

Yes, I think only supporting > 0.13 makes sense. I like your approach; if there're API differences we can add shim functions to the EsVersion class.

[phadej]

yes, the EsVersion is "API specification", which Backpack would do for us, but for now we have to go with classes :)

[phadej]

I'll try to make PR in few days

[arybczak]

class EsVersion ver where
  type EsUsername ver
  type EsPassword ver

data EsVersion1
data EsVersion5

I suggest using DataKinds for that.