Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

Adds support for write concern #2081

Merged
merged 1 commit into from Oct 7, 2015
Merged

Conversation

dkliban
Copy link
Member

@dkliban dkliban commented Sep 28, 2015

Pulp now accepts a write concern for the MongoDB connection. All writes
to the database are now acknowledged. For replica sets, 'majority' or 'all'
members can be asked to acknowledge the write.

@pulpbot
Copy link
Member

pulpbot commented Sep 28, 2015

Refer to this link for build results (access rights to CI server needed):
https://pulp-jenkins.rhev-ci-vms.eng.rdu2.redhat.com//job/unittest-pulp-pr/1397/

@@ -52,6 +52,10 @@ New Features
* Pulp properly connects to MongoDB replica sets. The `replica_set` setting in `database` section
of `/etc/pulp/server.conf` must be provided for Pulp to connect to a replica set.

* Pulp allows user to specify the write concern for MongoDB connection. The two options are
Copy link
Contributor

Choose a reason for hiding this comment

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

s/user/users/

Copy link
Contributor

Choose a reason for hiding this comment

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

s/for/for the/

@bowlofeggs
Copy link
Contributor

A few things to fix, and I'll re-review:

  1. We need to raise an Exception if the user is using mongo 2.4 and sets write concern without a replica set.

  2. We need unit tests that test all the cases, success and error. These tests didn't assert normal cases (mongo 2.4 with replica set, mongo 2.6 with replica set, mongo 2.6 without replica set, for example.)

  3. The test assertions need to be stronger. These tests only assert that an error was raised, but not that it was the correct error.

@dkliban
Copy link
Member Author

dkliban commented Sep 30, 2015

@rbarlow If a user is using Mongo 2.4, he does not have to use a replica set. Our default of 'majority' does not work without using a replica set. However, when MongoDB is running in single node mode, a write concern of 1 behaves the same way in MongoDB 2.4 as 'majority' in MongoDB 2.6.

@bowlofeggs
Copy link
Contributor

ok test

@bowlofeggs bowlofeggs assigned bowlofeggs and unassigned dkliban Oct 1, 2015
@dkliban
Copy link
Member Author

dkliban commented Oct 2, 2015

ok test

@pulpbot
Copy link
Member

pulpbot commented Oct 2, 2015

Refer to this link for build results (access rights to CI server needed):
https://pulp-jenkins.rhev-ci-vms.eng.rdu2.redhat.com//job/unittest-pulp-pr/1414/

@@ -100,6 +100,9 @@
PLP0041 = Error("PLP0041", _("Database 'replica_set' config must be specified when more than one "
"seed is provided. Refer to /etc/pulp/server.conf for proper use."),
[])
PLP0043 = Error("PLP0043", _("Database 'write_concern' config can only be 'majority' or 'all'. "
Copy link
Contributor

Choose a reason for hiding this comment

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

why not 42?

Copy link
Member Author

Choose a reason for hiding this comment

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

@mhrivnak Because we already have 42 on master

@pulpbot
Copy link
Member

pulpbot commented Oct 6, 2015

Refer to this link for build results (access rights to CI server needed):
https://pulp-jenkins.rhev-ci-vms.eng.rdu2.redhat.com//job/unittest-pulp-pr/1434/

@dkliban
Copy link
Member Author

dkliban commented Oct 6, 2015

ok test

@pulpbot
Copy link
Member

pulpbot commented Oct 6, 2015

Refer to this link for build results (access rights to CI server needed):
https://pulp-jenkins.rhev-ci-vms.eng.rdu2.redhat.com//job/unittest-pulp-pr/1439/

@dkliban dkliban assigned mhrivnak and unassigned dkliban Oct 6, 2015
if len(seeds_list) > 1 and not replica_set:
raise PulpCodedException(error_code=error_codes.PLP0041)
while True:
_CONNECTION = _connect_to_one_of_seeds(connection_kwargs, seeds_list, name)
if _CONNECTION:
db_version = semantic_version.Version(_CONNECTION.server_info()['version'])
if cmp(db_version, MONGO_MINIMUM_VERSION) == -1:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this (and below) would be more readable with normal comparison operators, which you can use with these objects (unless you found some edge case?). I suggest

db_version < MONGO_MINIMUM_VERSION

@dkliban
Copy link
Member Author

dkliban commented Oct 7, 2015

ok test

@pulpbot
Copy link
Member

pulpbot commented Oct 7, 2015

Refer to this link for build results (access rights to CI server needed):
https://pulp-jenkins.rhev-ci-vms.eng.rdu2.redhat.com//job/unittest-pulp-pr/1442/

@@ -1,18 +1,23 @@
import unittest
import unittest, unittest2
Copy link
Contributor

Choose a reason for hiding this comment

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

last comment, I swear. ;) unittest2 is a third-party module, so it should be in the second group of imports.

@mhrivnak mhrivnak added the LGTM label Oct 7, 2015
@mhrivnak mhrivnak assigned dkliban and unassigned mhrivnak Oct 7, 2015
Pulp now accepts a write concern for the MongoDB connection. All writes
to the database are now acknowledged. For replica sets, 'majority' or 'all'
members can be asked to acknowledge the write.

closes: pulp#974
closes: pulp#1139
@dkliban
Copy link
Member Author

dkliban commented Oct 7, 2015

ok test

@pulpbot
Copy link
Member

pulpbot commented Oct 7, 2015

Refer to this link for build results (access rights to CI server needed):
https://pulp-jenkins.rhev-ci-vms.eng.rdu2.redhat.com//job/unittest-pulp-pr/1444/

dkliban added a commit that referenced this pull request Oct 7, 2015
Adds support for write concern
@dkliban dkliban merged commit 15e9277 into pulp:2.7-testing Oct 7, 2015
ehelms pushed a commit to ehelms/pulp that referenced this pull request Aug 16, 2017
…nt changes

Updates a particular distributor's last published timestamp

closes pulp#2081
https://pulp.plan.io/issues/2081

(cherry picked from commit deabcbc)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants