Skip to content
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

Simplification of types, easier interop with other libraries #20

Merged
merged 7 commits into from
Apr 12, 2014

Conversation

snoyberg
Copy link

@snoyberg snoyberg commented Jan 2, 2014

I've been working on a simplified approach to setting up types and classes in persistent. In this process, I realized that the current setup of the mongoDB package will make such a rewrite impossible, since mongoDB forces usage of the Action transformer without exposing its internals.

I've worked on a fairly substantial overhaul of the types, which doesn't really change the inner operations or the user-facing API in a significant way. Some of the ideas behind this are described in my not-yet-published blog post on the subject:

https://www.fpcomplete.com/tutorial-preview/2896/z9jwaCGLzb

I'm not sure if all of the changes I've made here will be acceptable to you for merging, but I'd like to discuss two changes in particular:

  • Remove ErrorT from the transformer stack.
  • Instead of using an abstract Action, use a ReaderT Context type. Alternatively, if you want to keep the Action type, exposing its constructor so that persistent could treat it as a ReaderT.

Pinging @gregwebs.

@gregwebs
Copy link

gregwebs commented Jan 2, 2014

looks good to me. Append a ?w=q to avoid looking at all the indentation changes

@gregwebs
Copy link

gregwebs commented Jan 2, 2014

sorry, I meant ?w=1

@snoyberg
Copy link
Author

snoyberg commented Apr 1, 2014

Bumping this issue. persistent 2.0 will require some form of different API to make a mongoDB backend, I'd like to move ahead on figuring out how we can make that happen.

@gregwebs
Copy link

gregwebs commented Apr 1, 2014

@superbobry @knsd @selectel can you take a look?

@si14
Copy link

si14 commented Apr 1, 2014

@superbobry @knsd please take a look, +1

@superbobry
Copy link

Michael, I like the proposal, so +1 for merging this.

Me and @knsd are working on the new MongoDB driver, so now is the right time to discuss the bad parts in the existing API.

@gregwebs
Copy link

gregwebs commented Apr 2, 2014

I would like to help test the new driver. The only bad part for me is the delay mentioned in #18. And the only thing I really want to add is #14 an understanding of how to auto reconnect.

@knsd knsd merged commit a43c94f into selectel:master Apr 12, 2014
@knsd
Copy link

knsd commented Apr 12, 2014

Hello @snoyberg, sorry for delay. I agree with your ideas, and I've just merged your PR, unfortunately it contains some backward incompatible changes, so we need to update documentation and usage examples and release new version after that.

@snoyberg
Copy link
Author

Awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants