search

diversario / connect-mongostore

MongoDB session store for Connect/Express.
57 stars 16 forks source link

URL parser doesn't work with replica sets #18

Open rtgibbons opened 10 years ago

rtgibbons commented 10 years ago

Right it's using the node.js url module to parse it, with works for single hosts but doesn't work with replica sets.

Some Examples:

I'd suggest either using mongodb's url_parser or muri

What do you think?

diversario commented 10 years ago

I think you're right :) I'd go with mongo's own parser. I'm also planning to switching this module to use MongoClient so it supports connection string but I don't have the bandwidth at the moment. I'll try to update URL parser ASAP (or I can merge your PR if you make one!).

rtgibbons commented 10 years ago

Nice, I currently don't have the bandwidth either. I'll add this to the list of things to hack on though as there is a chance I'll be able to use this module more often.

seanmwalker commented 10 years ago

I've put something in that handles this. It does use mongodb's parse function and then maps the objects to this modules internal structure.

https://github.com/diversario/connect-mongostore/pull/25

It doesn't use MongoClient.connect yet, as that broke just about every test. I did have a hacked up version that used it in my first commit, and it does work. But it broke all of the other paths with config classes etc. This pull request at least seems to work with all of the existing approaches.

diversario commented 10 years ago

As I mentioned in the #25 there's a branch feature/use-mongoclient that breaks compatibility with current versions and uses MongoClient. I would really love for someone to try it in real life!

seanmwalker commented 10 years ago

Just a note, I tried the branch on our codebase where we use a replica set connection string. It worked fine for this app. I killed the primary and once the election completed it was responsive as expected. So that path works pretty well.