Skip to content
This repository has been archived by the owner on Nov 5, 2019. It is now read-only.

Add replicaset support #58

Merged
merged 1 commit into from
Apr 3, 2013
Merged

Conversation

manover
Copy link
Contributor

@manover manover commented Mar 7, 2013

ReplicaSet support

@manover
Copy link
Contributor Author

manover commented Mar 15, 2013

@jehiah @mreiferson @ploxiln
Basic RS support implemented, no working tests yet

- `dbuser`: db user to connect with
- `dbpass`: db password
- `autoreconnect` (optional): auto reconnect on interface errors
- `rs` (optional): replica set name (required when replica sets are used)
- `seed` (optional): seed list to connect to a replica set (required when replica sets are used)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to either a) make this a more structured [(host, port)...] list, or at least ['host:port', ...] or switch this to code that handles mongo uri's and have ['mongo://host:port',...]. In either case since it's a list of spots to connect to, let's make it a list instead of a comma separated string.

@jehiah
Copy link
Member

jehiah commented Mar 15, 2013

@manover this looks like a very good way to structure the authentication/replica set discovery flow. nice work so far.

logging.info("Waiting for %d to become master", port)
time.sleep(5)

def setUp(self):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to try using a setUpClass class method to do the expensive mongo setup instead of the setUp method. By using setUpClass your setup should only be run once for all of tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I could do that or even use a module-wide setup() function, the only problems are:

  1. It would still take a lot of time, even running once, so Travis would fail
  2. I would really hate to duplicate the existing structure used by other tests (with MongoTest as a super class)

but thanks for the tip

@ghost ghost assigned manover Mar 26, 2013
@manover
Copy link
Contributor Author

manover commented Mar 27, 2013

@ploxiln @jehiah @mreiferson Please review


def mongo_cmd(self, cmd, port=27018, res='"ok" : 1'):
pipe = subprocess.Popen("echo '%s' | mongo --port %d" % (cmd, port), shell=True, stdout=subprocess.PIPE)
repl = pipe.communicate()[0]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be:

mongo_cli = subprocess.Popen(["mongo", "--port", str(port)], stdin=subprocess.PIPE, stdout=subprocess.PIPE)
reply, _ = mongo_cli.communicate(cmd)

which I arbitrarily consider to be more elegant :)

@jehiah
Copy link
Member

jehiah commented Apr 3, 2013

@manover changes look great. go ahead and squash this change.

jehiah added a commit that referenced this pull request Apr 3, 2013
@jehiah jehiah merged commit 810e11b into bitly:master Apr 3, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants