Add com.snowplowanalytics.sauna.responders/SendgridConfig/avro/1-0-1

[rzats]

Add a second option to enable the RT command.

[alexanderdean]

This is an additive change on an existing released schema - should be a 1-0-1, not a patch...

[rzats]

@chuwy: does avro4s support multiple schema versions? Adding a 1-0-1.avsc schema to sauna/src/main/avro/com.snowplowanalytics.sauna.responders/SendgridConfig/avro/ results in this:

org.apache.avro.SchemaParseException: Can't redefine: com.snowplowanalytics.sauna.responders.SendgridConfig
        at org.apache.avro.Schema$Names.put(Schema.java:1060)
        at org.apache.avro.Schema$Names.add(Schema.java:1055)
        at org.apache.avro.Schema.parse(Schema.java:1187)
        ...

[chuwy]

I don't think so. And that is the problem!

@alexanderdean what options do we have?

• Add special syntax to denote SchemaVer in classes AmazonDynamodbConfig$1$0$1 (really ugly)
• Remove 1-0-0.avsc file from work tree and replace by 1-0-1.avsc
• Anything else?

@rzats could you please check if it possible to parse configuration of version 1-0-0 by sauna-0.2.0-SNAPSHOT with 1-0-1.avsc (hope this sentence makes sense).

[chuwy]

Technically, according to SchemaVer, for addition you always have a function 1-0-1 -> 1-0-0, for revision it's partial function 1-1-0 -> Option[1-0-0] and there's no meaningful 2-0-0 -> 1-0-0 for model. From this perspective you need to have both 1-0-0 and 1-0-1 avro schemas.

[alexanderdean]

From this perspective you need to have both 1-0-0 and 1-0-1 avro schemas.

Are you sure about this? A 1-0-1 reader can always consume a 1-0-0, so why would you need to have both schemas available?

Add special syntax to denote SchemaVer in classes AmazonDynamodbConfig$1$0$1 (really ugly)

Actually I quite like this! It's a very unopinionated representation of SchemaVer at the class level.

[rzats]

@chuwy: just tested this and no, when a 1-0-1 reader consumes a 1-0-0 configuration this case check isn't passed - I guess because params no longer matches SendgridConfigParameters.

I agree with Alex in that the class naming solution looks good (though a more readable separator would be nice - is $ standard for this sort of thing?)

[chuwy]

1. emailsEnabled is not nullable field. I don't think it matches addition anymore. Parser seeks for emailsEnabled, which is not present in 1-0-0 configs. Therefore params no longer matches SendgridConfigParameters.
2. If this is revision (or model) we really need both schemas, because we parse them separately, but for addition Alex is right, we can always have latest.
3. $ is valid symbol for Scala (and I think for JVM) whereas to use hyphens we'll need to add backticks which probably unrepresentable in Avro and Java. But we can try.

[alexanderdean]

Let's make emailsEnabled nullable then, then we have an addition and we only need to load the 1-0-1?

[rzats]

So default fields are supported by Avro and properly converted to case classes, e.g.

"name": "SendgridConfigParameters",
"type": "record",
"fields": [
  {
    "name": "recipientsEnabled",
    "type": "boolean"
  },
  {
    "name": "emailsEnabled",
    "type": "boolean",
    "default": false
  },
  {
    "name": "apiKeyId",
    "type": "string"
  }
]

becomes

case class SendgridConfigParameters(recipientsEnabled: Boolean, emailsEnabled: Boolean = false, apiKeyId: String)

but the problem is a config with a missing emailsEnabled field is still not recognised as a valid SendgridConfig. This might be a Play JSON-specific issue - I'll be looking into it some more.

[rzats]

The issue is caused by avro4s, which does not support optional field deserialization (even though it supports the reverse - i.e. default values in case classes are converted to defaults in the Avro schema). I'll create an issue in avro4s (EDIT: created at sksamuel/avro4s#106), but for now should I go for the class naming solution?

[alexanderdean]

Yep - class naming solution sounds best for now