Make it possible to get and set superseding schema info #130
Conversation
There was a problem hiding this comment.
This looks great.
I wondered if some of this should be implemented in iglu-scala-core instead of iglu-server. For example, the deserializers and serializers. Maybe the definition of SelfDescribingSchema needs to change? Might we need to use similar serializers/deserializers in igluctl?
But I'm also fine to merge this as-is, and then tackle it in iglu-scala-core at another time, only if needed.
| body JSON NOT NULL | ||
| body JSON NOT NULL, | ||
|
|
||
| supersededby VARCHAR(128) NULL |
There was a problem hiding this comment.
This is a nit, but superseded_by (with underscore) is more in-keeping with the other column names.
| for { | ||
| existing <- db.getSchema(schema.self).map(_.isDefined) | ||
| _ <- if (existing) db.updateSchema(schema.self, schema.schema, isPublic) | ||
| else db.addSchema(schema.self, schema.schema, isPublic) | ||
| _ <- updateSupersedingVersion(schema.self, s) |
There was a problem hiding this comment.
Have you considered what happens if addSchema is successful but updateSupersedingVersion is unsuccessful? It struck me that nothing runs inside a transaction.
I'm not saying this is a problem. Please just confirm the database can't end up in an inconsistent state?
There was a problem hiding this comment.
In that case, it would be like superseding info wasn't submitted at all. I don't think it would lead to inconsistent state.
I thought about it but I couldn't be sure if we should do it or not since |
Details are in #129