[TSD-39] Caching using a simple SQLite database #18
Conversation
HirotoShioi
left a comment
HirotoShioi
left a comment
There was a problem hiding this comment.
Currently reviewing. This is first time I've seen sqlite-simple being used so it'll take some time. But implementation seems straight-forward.
test/Spec.hs
Outdated
| , zlGetTicketInfo = \_ -> pure $ Just ticketInfo | ||
| , zlPostTicketComment = \_ -> pure () | ||
| , zlGetTicketComments = \_ -> pure comments | ||
| , zlGetAttachment = \_ -> pure mempty |
There was a problem hiding this comment.
Tests are failing here.
• No instance for (Monoid AttachmentContent)
arising from a use of ‘mempty’
src/DataSource/DB.hs
Outdated
| import Database.SQLite.Simple.FromField | ||
| import Database.SQLite.Simple.Internal | ||
| import Database.SQLite.Simple.Ok | ||
| import Database.SQLite.Simple.ToField |
There was a problem hiding this comment.
Can you make these imports explicit?
src/DataSource/DB.hs
Outdated
|
|
||
| instance FromField TicketId where | ||
| fromField (Field (SQLInteger tId) _) = Ok . TicketId . fromIntegral $ tId | ||
| fromField f = returnError ConversionFailed f "need a integer, ticket id" |
There was a problem hiding this comment.
Grammar error. "need an integer, ticket id" . Other error message has same issues as well
There was a problem hiding this comment.
Interesting, I remember I read somewhere they can be interchanged, but it doesn't seem to be so - https://www.quickanddirtytips.com/education/grammar/a-versus-an
| TicketTags (..), TicketURL (..), UserId (..), ZendeskLayer (..)) | ||
|
|
||
|
|
||
| -- https://ocharles.org.uk/blog/posts/2014-08-07-postgresql-simple-generic-sop.html |
There was a problem hiding this comment.
Not sure what does meant. I read the article and it seems like you can remove all these typeclass definitions. Is this for the future task?
There was a problem hiding this comment.
Yes, that is why I pasted it here.
|
Also, I'm not really sure how we're going to utilize user datatype but I guess that'll be clarified in the next PR |
|
We should add |
|
*.db will be created in the next PR along with the git ignore rule. The rest is ok. |
robcohen
left a comment
robcohen
left a comment
There was a problem hiding this comment.
I do not see any issues in this PR.
https://iohk.myjetbrains.com/youtrack/issue/TSD-39
A read-only implementation of the SQLite DB, still missing the write from web API side, but since this is already a large PR, will fix in another one.