Aeson-2.x compatibility and fix for new Read(UTCTime)

[pbrisbin]

This gets things building and passing with lts-20.05 / GHC-9.2.5

Aeson-2.x compatibility

Relax UTCTime parser

It seems with the newer time package, reads is returning more than a
single element tuple,

λ> t = "2009-04-15 10:02:06 UTC"
λ> reads @UTCTime t
[(2009-04-15 10:02:06 UTC,""),(2009-04-15 10:02:06 UTC," UTC")]

Checking for a first element with no trailing characters gets the tests
passing again.

Tangential: there seems to be a lot more manual processing of the
Yaml than I'd expect. Why does a Migration file not just have a clear
record type along with a FromJSON and then you just decode it
(letting aeson handle all error reporting)? Is there some scar tissue and/or
context I'm missing?

[jtdaugherty]

there seems to be a lot more manual processing of the Yaml than I'd expect

Do you mean JSON here, not Yaml?

As for why there isn't a FromJSON instance, at this point I don't remember. It's possible that I just didn't know about FromJSON at all at the time or chose not to use it (since using type classes like that is not always the right way to go).

There is indeed a record type for Migration although in practice there is not always a 1-to-1 correspondence between such types and a JSON representation (which is one situation in which a FromJSON is not appropriate). In any case, this package is old, the context is fuzzy since it's been over a decade since I wrote it, and I no longer use or maintain it. I am still happy to consider patches to it, but I just added a note to the README about this.

[pbrisbin]

processing of the Yaml
Do you mean JSON here, not Yaml?

I guess I meant "processing of Yaml [files, using JSON instances]"

You are doing that here, you're just decoding as Object and then manually handling the values. You can get all that for free by just giving Migration a FromJSON and decoding to that type directly. The decoding will fail on missing keys (for non-Maybe fields), unrecognized keys, or an invalid UTCTime. You used to have to write the decode by hand, and ensure all this behavior (which would've still been better, IMO), but just using the Generic machinery we have now should behave as you need, as far as I can tell.

instance FromJSON Migration where
  parseJSON = genericParseJSON options
    where
      
options = -- this is used to say to parse "Depends" for "mDepends", to error on unknown fields, etc

-- Then it's just...
eMigration <- Yaml.decodeFileEither yaml

It seems like it's just lost to history why it was done this way (my guess is it had to be with yaml-light and then was just retained across the transition to yaml).

There is indeed a record type for Migration although in practice there is not always a 1-to-1 correspondence between such types and a JSON representation

Yeah, that'll happen. The idiomatic approach for that is to define a new MigrationJSON which is 1-to-1, decode at that, and then use some MigrationJSON -> Migration function to resolve the differences. Sometimes it can be good to do this up-front, to give yourself that seam, but I find it's no harder to add later, whenever that need arises. At first glance, I don't know that you'd need it here.

Anyway, I can follow up in a new PR so you can see all this concretely -- now that I know the status quo is not intentional.

[pbrisbin]

I can follow up in a new PR so you can see all this concretely

Actually, doing that would resolve both bugs this PR is resolving incidentally and more cleanly. Perhaps I should do that instead?

[jtdaugherty]

Actually, doing that would resolve both bugs this PR is resolving incidentally and more cleanly. Perhaps I should do that instead?

I don't have a preference in this case.

[pbrisbin]

Cool, I opened #43 instead.