New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a class for backend compatibility #53

Merged
merged 13 commits into from Feb 27, 2018

Conversation

Projects
None yet
3 participants
@parsonsmatt
Copy link
Collaborator

parsonsmatt commented Sep 13, 2017

This PR will enable Esqueleto queries to be used by the persistent-typed-db library, along with other database backends.

This is a WIP, please don't merge until I've had time to use it in-anger.

parsonsmatt added some commits Sep 13, 2017

@parsonsmatt parsonsmatt changed the title WIP: Add a class for backend compatibility Add a class for backend compatibility Oct 23, 2017

@parsonsmatt

This comment has been minimized.

Copy link
Collaborator

parsonsmatt commented Oct 23, 2017

This PR now uses the backend compatibility class introduced in persistent-2.7.1 instead of defining it here.

The work project "just worked" with this patch and the new persistent, so I suspect it's fine. I'll try to find someone with heavier usage of the SqlReadT stuff to see if it is OK for them.

parsonsmatt added some commits Oct 23, 2017

@parsonsmatt

This comment has been minimized.

Copy link
Collaborator

parsonsmatt commented Oct 24, 2017

I've opened this PR in persistent that should make this a non-breaking change for existing projects. Once that's reviewed/merged, I'll bump the lower bounds on this library, use the hackage version, and we'll be in business :)

@dariooddenino

This comment has been minimized.

Copy link

dariooddenino commented Oct 25, 2017

Using both this PR and the persistent one you linked, I still get this error:

 • No instance for (BackendCompatible SqlBackend (SqlFor LivDB))
        arising from a use of ‘E.select’
    • In a stmt of a 'do' block:
        E.select
        $ E.from
          $ \ p
              -> do { E.where_ (p E.^. Wp_postmetaMeta_id E.==. E.val x);
                      return p }
      In the second argument of ‘($)’, namely
        ‘do { E.select $ E.from $ \ p -> do { ... } }’
      In the expression:
        runLiv $ do { E.select $ E.from $ \ p -> do { ... } }

Persistent queries work correctly. It's a mysql db on persistent-mysql-2.7.1

@parsonsmatt

This comment has been minimized.

Copy link
Collaborator

parsonsmatt commented Oct 25, 2017

@dariooddenino Try with latest commit of persitent-typed-db -- I forgot to move the instance into the library :)

@dariooddenino

This comment has been minimized.

Copy link

dariooddenino commented Oct 26, 2017

persistent-typed-db last commit fixed this :)

Just a note: valkey still uses toSqlKey instead of toSqlKeyFor

@dariooddenino

This comment has been minimized.

Copy link

dariooddenino commented Jan 4, 2018

Does this works for select only? If I try to use update I get the Couldn't match type 'SqlFor Foo' with 'SqlBackend' error.
Let me know if you need more info :)

parsonsmatt added some commits Jan 17, 2018

@parsonsmatt

This comment has been minimized.

Copy link
Collaborator

parsonsmatt commented Feb 16, 2018

@dariooddenino I have added tests for the persistent-typed-db library that ensure that it type checks with updates and selects now :)

@bitemyapp This is working and relying on Hackage persistent. However, upstream persistent bundles along new Conduit and a bunch of other breaking changes. Merging and releasing this would be a major bump probably. Thoughts?

@bitemyapp

This comment has been minimized.

Copy link
Owner

bitemyapp commented Feb 16, 2018

@parsonsmatt I lean towards doing major bumps whenever in doubt. I will have time this weekend to help with this too.

@parsonsmatt

This comment has been minimized.

Copy link
Collaborator

parsonsmatt commented Feb 16, 2018

Cool. If you're good with a major bump for this, then Im happy with that too. Anything we want to get in with this version?

@bitemyapp

This comment has been minimized.

Copy link
Owner

bitemyapp commented Feb 16, 2018

@parsonsmatt other than any relevant persistent and conduit upgrades? Not that I'm aware of off-hand but I'll re-peruse the diff and be more certain after I've done so. You got anything in mind?

@parsonsmatt

This comment has been minimized.

Copy link
Collaborator

parsonsmatt commented Feb 16, 2018

Idk, there's probably some deprecations or stuff that can make the interface better?

@bitemyapp bitemyapp merged commit 86c4c1a into bitemyapp:master Feb 27, 2018

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@bitemyapp

This comment has been minimized.

Copy link
Owner

bitemyapp commented Feb 27, 2018

@parsonsmatt I have some work to do yet on cleaning up the test suite but this is now merged. Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment